Skip to content

Add new dockerfile images and reqs for celery#388

Open
IMBCIT wants to merge 4 commits intomasterfrom
docker-refactor
Open

Add new dockerfile images and reqs for celery#388
IMBCIT wants to merge 4 commits intomasterfrom
docker-refactor

Conversation

@IMBCIT
Copy link
Contributor

@IMBCIT IMBCIT commented Jul 3, 2023

This is to reduce the requirements and size of the docker containers we are using to save some space.

Copy link
Collaborator

@ezio-melotti ezio-melotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should some of these be removed from the requirements.txt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add the step process back in.


COPY ./requirements.txt /simoc/requirements.txt
RUN python3 -m pip install -r /simoc/requirements.txt
curl
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove it from the celery containers as it should not be needed in theory.

@ezio-melotti ezio-melotti changed the title adding new dockerfile images and reqs for celery Add new dockerfile images and reqs for celery Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants