-
-
Notifications
You must be signed in to change notification settings - Fork 314
Add docker compatibility #252
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
Conversation
28c43c8 to
b7be144
Compare
docker-compose.yml
Outdated
| build: . | ||
| image: expdev07/coronavirus-tracker-api:2.0 | ||
| restart: always | ||
| command: pipenv --python 3.7 run dev |
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.
We shouldn't be running the dev version with the "reload" flag in the container.
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.
Done!
Pipfile
Outdated
|
|
||
| [scripts] | ||
| dev = "uvicorn app.main:APP --reload" | ||
| dev = "uvicorn --host 0.0.0.0 app.main:APP --reload" |
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 shouldn't be changing this.
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.
Done!
b7be144 to
d4d7d2d
Compare
Dockerfile
Outdated
|
|
||
| # INSTALL DEPENDENCIES | ||
| RUN pip install pipenv | ||
| RUN pipenv install --system --deploy --dev |
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.
We should install from the requirements files once this goes in. #245
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.
Which requirements file? requirements-dev.txt or requirements.txt?
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.
Just requirements.txt
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.
Done!!
Kilo59
left a comment
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.
use requirements.txt
d4d7d2d to
5b0631e
Compare
5b0631e to
4ffe433
Compare
Notes:
The pipenv is slow down the image build, also, increase ~37mb of image's size but this pull request is just to add docker flow for development enviroment. To run on production enviroments safety, we will need to do somes ajusts that could change the project a little bit.
The app waits a environment variable called "PORT" but, even without it, i can run the api using the port 8000 (a little bit strange).
#252