Reduce docker image size #1648
Reduce docker image size #1648
Conversation
|
Here I am, brain the size of a planet and they ask me to welcome you to Prefect. So, welcome to the community @wagoodman! 🎉 🎉 |
wagoodman
force-pushed
the
1638-optimize-docker-image
branch
2 times, most recently
from
3c91713
to
0e2a95e
Compare
wagoodman
force-pushed
the
1638-optimize-docker-image
branch
from
0e2a95e
to
3aad2e7
Compare
| command: | | ||
| docker login --username $DOCKER_HUB_USER --password $DOCKER_HUB_PW | ||
| docker push prefecthq/prefect | ||
| docker push prefecthq/prefect:${CIRCLE_TAG}-${PYTHON_TAG} |
There was a problem hiding this comment.
I think we might want to finesse the logic here a little, because I suspect this will end up pushing an image with a messy git tag
There was a problem hiding this comment.
I'm open to a better way to do it. I added circle-ci git tag filters here to help reduce the risk of an odd tag coming through.
There was a problem hiding this comment.
Hmmm yea but won't this job still run on master?
There was a problem hiding this comment.
good call, I thought these were anded instead of ored. I'll update...
There was a problem hiding this comment.
Interestingly enough there isn't a good way to do this, @TylerWanner and I came up with this combo:
- https://github.com/PrefectHQ/prefect/pull/1648/files#diff-1d37e48f9ceff6d8030570cd36286a61R361-R364
- https://github.com/PrefectHQ/prefect/pull/1648/files#diff-1d37e48f9ceff6d8030570cd36286a61R286-R292
Up to a better approach, but this is all I've got until circleci supports and'ed filters.
There was a problem hiding this comment.
I think this is the only place I'm still hesitant; here's what I imagine happening:
- a PR is merged to master
build_docker_imageis runCIRCLE_TAGis empty, because this isn't a tagged commit- either this step or the build step errors out in some unique way based on this
Everything else looks great though! And to be honest, we could always clean things up if that happens but I want to make sure I understand this piece
There was a problem hiding this comment.
I don't believe that it is possible to reach this step because of the workflow filter, however, I'm all on-board for an extra measure of safety. Just added the set -u mentioned below which will fail the build image step on empty env vars.
There was a problem hiding this comment.
Python-wise I'm feeling good about this - @TylerWanner would be interested to hear your thoughts on the circle changes
| assert not storage.env_vars | ||
| assert storage.python_dependencies == ["wheel"] | ||
| assert storage.env_vars == { | ||
| "PREFECT__USER_CONFIG_PATH": "/root/.prefect/config.toml" |
There was a problem hiding this comment.
@joshmeek do you remember why we had to include this originally? I suspect it's no longer necessary but I don't want to take any chances
There was a problem hiding this comment.
don't remove it as part of this PR, but do remove it as part of another PR after this gets merged pls
wagoodman
force-pushed
the
1638-optimize-docker-image
branch
from
5852518
to
d183aeb
Compare
wagoodman
force-pushed
the
1638-optimize-docker-image
branch
from
d183aeb
to
613713b
Compare
Co-Authored-By: Josh Meek <40716964+joshmeek@users.noreply.github.com>
wagoodman
force-pushed
the
1638-optimize-docker-image
branch
from
d65e5f0
to
c8b9007
Compare
Optimize Dask task runner future resolution
Thanks for contributing to Prefect!
Please describe your work and make sure your PR:
CHANGELOG.md(if appropriate)docs/outline.tomlfor API reference docs (if appropriate)Note that your PR will not be reviewed unless all three boxes are checked.
What does this PR change?
python:*-slimimages (addressing Optimize our Dockerfiles #1638 ).Why is this PR important?
This cuts down the time it takes to pull down the prefect image for the first time, thus getting users to their first flow faster.