Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Added configuration for using specific SQLite versions. #50
base: main
Are you sure you want to change the base?
Added configuration for using specific SQLite versions. #50
Changes from all commits
f5b0fc2
73e79db
7a6c654
8ef6e51
1ab68b9
e2f1dd7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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 don't push the image to Docker Hub. This ensures doing
FROM django-docker-box:${PYTHON_IMPLEMENTATION}-${PYTHON_VERSION}
won't make Docker try to pull it from Docker Hub if the image isn't available locally.Could also try
build
instead, but if I recall correctly it doesn't work in this context unless you already built the base image. Usingbuild
is even worse as it will run the build process even if the image has already been built. It does use the cache, but it also adds a few seconds to the run, compared tonever
that would just skip it.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.
I think there might be something off here. Here's what I get trying to build the image against Docker 4.38
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.
Ahh I see. I need to build the base first with
PYTHON_VERSION=3.12 docker compose build base
.Might be worth documenting.
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.
I investigated how we could define x-service image dependencies and I stumbled upon this page (which was deleted not too long along to encourage users to move to
bake
).Unfortunately the solution they propose doesn't work when the image that you depend upon is parametrized
The only solution I could find was to use
depends_on
to ensure the image was successfullyThere 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.
It is also mentioned in these docs
But I can't get it work either with
It results in
Which is unsurprising as even their minimal example fails to run (when fixing their duplicate
base
service name 🤦 )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.
Thanks for looking into this, Simon!
I've refactored the setup to use multi-stage build in 1ab68b9.
It allows us to reuse the layers from the base image. Unfortunately, they're treated as two completely different images though, so Docker will still run all the steps from scratch instead of just the steps from after the base image. It's fairly quick as the layers will be reused (as long as it doesn't think the cached apt install layer didn't miss...), but technically it can be made faster by somehow using the built image.
For example, the following trick seems to work last time I tested it:
but I'm not sure if that'd be acceptable. The
ARG BASE_IMAGE=pass
in the base image is just a placeholder as otherwise Docker would complain if an initial value is not provided, despite only being used for the SQLite image.Also that approach would couple the
Containerfile
with thecompose.yml
file unless you provideBASE_IMAGE
yourself. Without this optimisation, theContainerfile
can be used standalone (I think) if one so wish.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.
This requires re-building
base
first otherwise you'll run into/bin/sh: 1: tclsh: not found
.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.
Yep – I wasn't sure what to do about it. Adding a note in the README seems redundant if it's only needed once if you've already built the image before.
Although after refactoring it to the multi-stage build, this is no longer the case as they're now two separate images (unless we use the optimisation trick I described in #50 (comment)).
I've verified this by deleting all existing images and containers, rebuilding from current
main
, and then only running something likeSQLITE_VERSION=3.41.0 SQLITE_CFLAGS="-DSQLITE_OMIT_JSON" docker compose run --rm sqlite db_functions.json
without rebuildingbase
.