-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add basic Docker image #9
base: master
Are you sure you want to change the base?
Conversation
Fixes #8 I'm doing something wrong linking the issue to the PR 😄 |
I think its closes #8. mabey edit into the description of the original one |
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.
Sorry to keep you waiting for so long, I had a rough week.
I've been trying to learn some more about docker to have some knowledge about how this is used and that's some cool stuff however I am unsure about few things and would like you to have a look at my comments.
npm install [email protected] && \ | ||
npm install [email protected] | ||
|
||
ENV HOST 0.0.0.0 |
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.
Shouldn't this be defaulted to 127.0.0.1
to keep consistency with config? I prefer localhost over this solution to not open our app to whole internet and manage its access via proxying port 7777 with stuff like nginx.
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.
This would be controlled by the ports you set with the docker commands. By default no ports are exposed. You can still only open to localhost by binding 127.0.0.1:7777
or to all interfaces with 7777:7777
.
If this is HOST 127.0.0.1
then you can only connect to it from inside the container.
Would you mind to elaborate on what code changes would be needed? Maybe I could tweak some things to make haste run better with docker |
@niekcandaele any chance of a refresh? |
Sorry for dissapearing here, got really busy and lost track of this
I was thinking mostly configuration via env vars instead of a config file. This current version will work if you add the config file via volumes but for containers it's generally easier to use env vars.
Dockerhub can also automatically build images on git push. Actions are faster but require a bit more setup. |
Adds a really basic Docker image. Improvements are definitely possible but would require more code changes I believe.
Currently installs all the storage peer dependencies, there's probably a better way to handle that. Separate images perhaps?
Has a basic docker-compose file as well which is set up to use the local file storage. Uses a volume for persistence.
Closes #8