-
Notifications
You must be signed in to change notification settings - Fork 0
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
Dockerize signsfive-api #22
Conversation
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.
Overall, most of the changes look relatively straightforward...
docker-compose.yml
Outdated
- ./node_modules:/home/node/app/node_modules | ||
- ./scripts:/home/node/app/scripts | ||
- ./src:/home/node/app/src | ||
- ./.npm:/home/node/.npm |
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.
do we need a .npm
volume? it's in the .gitignore
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.
Not necessary but if want able to view the log, this is easiest way unless you want to use docker exec bash
UPDATE: I just realized. We will want that to only happen in during development mode not production. I will make edit on docker-compose.yml and docker-compose.dev.yml 👍
docker-compose.yml
Outdated
- ./package-lock.json:/home/node/app/package-lock.json | ||
- ./server.log:/home/node/app/server.log | ||
ports: | ||
- 8999:8282 |
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.
how did you decide on the ports?
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 is now in .env
and only applies to development mode.
docker-compose.yml
Outdated
command: npm start | ||
networks: | ||
signsfive_net: | ||
ipv4_address: 172.28.1.2 |
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.
how did you decide on ip address?
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 is now in .env
and only applies to development mode.
docker-compose.yml
Outdated
depends_on: | ||
- database | ||
ports: | ||
- 9992:80 |
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.
how did you decide on port?
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 is now in .env
and only applies to development mode.
I will update documentation about how running in docker and what to do with |
This is now out of WIP and PTAL! |
README.rst
Outdated
@@ -26,6 +26,8 @@ Next, both ``docker-compose.yml`` and ``docker-compose.dev.yml`` are required, s | |||
|
|||
npm run dev | |||
|
|||
NOTE: This might take signsfive_api to restart couple of time due to race condition with database starting up. |
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 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.
/issues/26 will try to resolve this race conditions
Issue has been open to deal with Docker's race condtions - /issues/26 |
I think this is good to be merged in? I made issue to deal with race condtions. Unless it will really break heroku? @kratsg |
Feature/babelize
This is partly working. But opened PR for purpose of review.
.env
aren't currently in used since I want to reduce complicated of changes as much as possible.