-
Notifications
You must be signed in to change notification settings - Fork 14
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
Improve Docker support #15
base: main
Are you sure you want to change the base?
Conversation
… config paths, add arm64 build
I was curious: Removing Playwright as a dependency (and the plugin that depends on it) brings the container size down to 141mb (from ~2gb), but obviously removes support for steamdb which is probably unwanted 😛 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forgot to change each plugin's config path as well
For the playwright part, I don't think it is neccessary to use docker if steamdb plugin is not needed. The only reason for this docker is to support for playwright's complicated requirements. I will help test CI build. |
Oops! Weirdly it still worked for me, and I'm not sure why. Python was being very strange with the current working directory - I'll pull it up in a debugger and see if I did something silly.
That's fair. I run all my services in docker just for convenience, which is why I modified it to work a bit more generically rather than being focused around playwright. Would you accept a second Dockerfile (maybe built with a different tag?) that removes the steamdb plugin and playwright dependency, since you don't recommend using it anyway (according to your readme)?
Thanks! Cheers for bearing with me - I'm not super familiar with python or Travis, but thought the changes I made to get this running on my pi might be useful to others. |
Of course, that is a good idea. You can use other base image for that. |
I added an env var that tells Gameshub to either use its src directory or the current working directory for config files, and I think I updated all references. I think that should work. I've tested both Dockerfiles and running the app outside of Docker and get the same output on both ARM+AMD64 (with no errors), but it's a bit hard to tell if it's working since the application is pretty quiet when there's nothing to do. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few issues.
- When I tested the image with CI, it appeared that something went wrong while building docker. Can you please help check it out? (Testing branch: test) Error Log
- In all plugins, you should not change
config_example_path = os.path.join(os.path.split(os.path.realpath(__file__))[0], "config.example.json5")
toconfig_example_path = os.path.join(GAMESHUB_SRC_DIR, os.path.split(os.path.realpath(__file__))[0], "config.example.json5")
. This is
deliberately using the current file's folder. - (Not a big issue.) It is not necessary to use all requirements.txt files from all plugins. The requirements.txt file at the top level contains all the required packages. Though no harm will be done,
Both docker build error can be recreated in my Ubuntu VM.
|
Hi, any updates? |
Hi,
This PR adds some Docker best-practice changes to:
arm64
variant (I wanted to run it on my Pi). Note I haven't tested the Travis build as I'm not familiar with it.For some reason I couldn't get the config to read from the current working directory within the container, so I added an env var to set the src location within the container, defaulting to
.
(current working directory) if not set, which I believe should work. My python isn't great, so please feel free to let me know if I made a mistake.