Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added environment variables to control bind address #23

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sorucoder
Copy link

Fulfilling wishes of #19.

This PR should allow to set the host and port of the webserver like so:

docker run -e HOST=127.0.0.1 -e PORT=80 quay.io/invidious/youtube-trusted-session-generator:webserver

I can't test it in docker due to unfamiliarity with it but I know my shell scripting. hope this helps.

# Run python script on display 0
DISPLAY=:99 python potoken-generator.py --bind 0.0.0.0
DISPLAY=:99 python potoken-generator.py --bind $HOST --port $PORT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no default value for $HOST. There is a need to have 0.0.0.0 as the host address.

Either you define this default value in the bash script or you pass it in the Dockerfile.

Copy link
Author

@sorucoder sorucoder Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed via commit 778648d.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will.

However, isn't that a bash only feature? I thought sh doesn't support that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ha yes. Feel free to switch the shabang to ash since this is based on alpine.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with commits 95d0d90 and f8c1de3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants