Conversation
ezio-melotti
left a comment
There was a problem hiding this comment.
I fixed a conflict with the .dockerignore and left a few comments about the rest.
| redis==4.5.4 | ||
| scikit-learn==1.2.0 | ||
| scipy==1.9.3 | ||
| sqlalchemy==1.4.46 No newline at end of file |
There was a problem hiding this comment.
Should some of these be removed from the requirements.txt?
There was a problem hiding this comment.
I played around with these dependencies quite a bit and if I remove anything else from either file something in the program will not function due to missing dependencies.
|
|
||
| COPY . /simoc | ||
|
|
||
| RUN pip install -r /simoc/requirements.txt |
There was a problem hiding this comment.
I think this was done in separate steps (first copy the requirements, then install them, then copy the rest), so that changing the dependencies can be cached when files change in the cwd. That is, I believe that without the extra step, if a file is added/changed, Docker has to execute the COPY step again and then reinstall the dependencies.
There was a problem hiding this comment.
I will add the step process back in.
Dockerfile-celery-worker
Outdated
|
|
||
| COPY ./requirements.txt /simoc/requirements.txt | ||
| RUN python3 -m pip install -r /simoc/requirements.txt | ||
| curl |
There was a problem hiding this comment.
Is curl needed in the celery container? AFAICT it's only used for the healthcheck of the flask container. If it's not needed we might be able to get rid of this step altogether.
There was a problem hiding this comment.
We can remove it from the celery containers as it should not be needed in theory.
This is to reduce the requirements and size of the docker containers we are using to save some space.