Skip to content
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

Reduce Docker image size #113

Merged
merged 2 commits into from Apr 6, 2022
Merged

Reduce Docker image size #113

merged 2 commits into from Apr 6, 2022

Conversation

mfitz
Copy link
Contributor

@mfitz mfitz commented Apr 6, 2022

Before

REPOSITORY               TAG        IMAGE ID       CREATED             SIZE
genet-local-master       latest     3e95afde36e9   30 seconds ago      1.75GB

After

REPOSITORY            TAG        IMAGE ID       CREATED             SIZE
genet-local           latest     61c9395a63bf   26 minutes ago      1.46GB

Still pretty chonky, even after reclaiming 300 megs. Questions @KasiaKoz:

  • Do we still need that Node installation? I think it was for something to do with Kepler, right?
  • notebooks/7. Visualising Network.ipynb is over 40 MB in size - is that expected? Seems outrageously big for a notebook.

Validating the image

I connected to the container and ran both the unit tests and the notebook smoke tests. The smoke tests all passed, but one unit test failed. However, when I ran the unit tests in the container created from the image in master, I saw the same failure, so this does not seem to have been caused by my changes.

Screen Shot 2022-04-06 at 02 23 04

Screen Shot 2022-04-06 at 02 16 25

@mfitz mfitz requested review from KasiaKoz and ana-kop April 6, 2022 01:47
Copy link
Collaborator

@KasiaKoz KasiaKoz left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for de-chonkifying genet, doing Gods work here @mfitz

We do still need node, kepler is still a functionality.. and you can generate and save those plots to html so it's possible someone would use that via docker.

Now onto the failing test in docker. Wawaweewa 👀 It looks like the snap and route functionality / maybe the max stable set optimisation gives different results inside---but why and what are the consequences 👀 a bit worried tbh. Should be investigated and I feel like we should have some automatic testing within docker... we do use genet within docker frequently 😰

@mfitz
Copy link
Contributor Author

mfitz commented Apr 6, 2022

Awesome, thanks for de-chonkifying genet, doing Gods work here @mfitz

We do still need node, kepler is still a functionality.. and you can generate and save those plots to html so it's possible someone would use that via docker.

Now onto the failing test in docker. Wawaweewa 👀 It looks like the snap and route functionality / maybe the max stable set optimisation gives different results inside---but why and what are the consequences 👀 a bit worried tbh. Should be investigated and I feel like we should have some automatic testing within docker... we do use genet within docker frequently 😰

We can add testing inside the container to the CodeBuild pipeline, we have a mechanism to do that by just adding a run-tests.sh to the root of the project - see here. This will fail the build right now though, so we should figure out what is going wrong with that test first.

@mfitz mfitz merged commit 68302b9 into master Apr 6, 2022
@mfitz mfitz deleted the shrink_docker_image branch April 6, 2022 10:28
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.

None yet

2 participants