-
Notifications
You must be signed in to change notification settings - Fork 41
Added configuration for using specific SQLite versions. #50
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
base: main
Are you sure you want to change the base?
Changes from all commits
f5b0fc2
73e79db
7a6c654
8ef6e51
1ab68b9
e2f1dd7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,7 @@ Tooling and test execution support for [Django][0] :unicorn: | |
3. Build the image: | ||
|
||
```console | ||
$ docker compose build sqlite | ||
$ docker compose build base | ||
``` | ||
|
||
4. Run the tests: | ||
|
@@ -204,7 +204,13 @@ The versions of various backend services can be switched by setting these enviro | |
| `ORACLE_VERSION` | `23.5.0.0` | Version of Oracle container image to use | | ||
| `POSTGRESQL_VERSION` | `14` | Version of PostgreSQL container image to use | | ||
| `POSTGIS_VERSION` | `3.1` | Version of PostGIS extension to use | | ||
| `SQLITE_VERSION` | | Version of SQLite to compile and use | | ||
|
||
> [!NOTE] | ||
> | ||
> If left unspecified, the SQLite version provided by Debian will be used. | ||
> Using a specific SQLite version requires compiling it from source. For more | ||
> details, see [SQLite Versions](#SQLite-Versions). | ||
|
||
### Python Versions | ||
|
||
|
@@ -229,7 +235,8 @@ restrictions with respect to the range of versions available. | |
### Database Versions | ||
|
||
Most database container images are pulled from [Docker Hub][2]. Oracle database | ||
is pulled from the [Oracle Container Registry][3]. | ||
is pulled from the [Oracle Container Registry][3]. Specific versions of SQLite | ||
are compiled directly from the tags in the [official Git mirror][11]. | ||
|
||
You can switch the version of the database you test against by changing the | ||
appropriate environment variable. Available options and their defaults can be | ||
|
@@ -273,6 +280,50 @@ To determine what database versions can be used you can check the release notes | |
for the branch of Django that you have checked out, or alternatively there is | ||
the [supported database versions][4] page on Django's Trac Wiki. | ||
|
||
#### SQLite Versions | ||
|
||
SQLite is normally bundled in the Python installation using the version | ||
available on the system where Python is compiled. We use the Python Docker image | ||
based on Debian `bookworm`, which has SQLite 3.40.1. | ||
|
||
To use a different version, we compile SQLite from source and load the library | ||
dynamically using `LD_PRELOAD`. There are a few caveats as a result: | ||
|
||
- Some SQLite features are only available if certain flags are set during | ||
compilation. SQLite is known to change these flags in newer releases, such as | ||
to enable features by default that were previously opt-in. When Python is | ||
compiled, it inspects the system's SQLite to determine features that are | ||
included in the `sqlite` module. A mismatch in the module and the dynamically | ||
loaded library may result in Python failing to load, which may happen if we | ||
use an SQLite version that is older than the system version. | ||
- Debian and Ubuntu use a custom `CFLAGS` variable to compile their distributed | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
SQLite. Historically, Django's CI has only been configured with SQLite | ||
versions that come with the operating system. If SQLite is compiled with | ||
different flags, some tests may fail. | ||
|
||
We currently work around the above caveats by setting the simplest `CFLAGS` | ||
value that allows all the tests to pass. To customize the `CFLAGS` used for the | ||
compilation, you can set the `SQLITE_CFLAGS` environment variable. See the | ||
[`.env`][10] file for its default value. | ||
|
||
``` | ||
SQLITE_VERSION=3.48.0 SQLITE_CFLAGS="-DSQLITE_OMIT_JSON -DSQLITE_MAX_VARIABLE_NUMBER=999" docker compose run --build --rm sqlite | ||
``` | ||
|
||
> [!NOTE] | ||
> | ||
> The `--build` argument is necessary if you've changed `SQLITE_CFLAGS` since | ||
> the last run, as it's not part of the image tag. You can also rebuild the | ||
> image separately by running `docker compose build sqlite`, optionally with | ||
> `--no-cache` to ignore the cached build. | ||
|
||
In the future, the Django codebase may be more robust when tested against | ||
different SQLite configurations and the `CFLAGS` workaround may no longer be | ||
necessary. | ||
|
||
Running SpatiaLite tests against specific versions of SQLite/SpatiaLite is not | ||
currently supported. The versions of SQLite and SpatiaLite that come with the | ||
operating system are used for these tests. | ||
|
||
### Other Versions | ||
|
||
|
@@ -294,7 +345,7 @@ with no promises that they'll be delivered: | |
- Add support for running accessibility tooling and report generation | ||
- Support report generation during monthly runs and publish to GitHub Pages | ||
- Publish pre-built container images to the GitHub Container Registry | ||
- Support testing against different versions of SQLite and SpatiaLite | ||
- Support testing against different versions of SpatiaLite | ||
- Support running with Podman in addition to Docker | ||
- Support generating screenshots into `./output/screenshots/` | ||
|
||
|
@@ -309,3 +360,5 @@ with no promises that they'll be delivered: | |
[7]: https://docs.djangoproject.com/en/stable/internals/contributing/writing-code/unit-tests/#running-the-unit-tests | ||
[8]: https://docs.djangoproject.com/en/stable/internals/contributing/writing-code/unit-tests/#running-the-selenium-tests | ||
[9]: https://docs.djangoproject.com/en/stable/ref/contrib/gis/testing/#geodjango-tests | ||
[10]: .env | ||
[11]: https://github.com/sqlite/sqlite |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,10 @@ | |
|
||
x-base: &base | ||
image: django-docker-box:${PYTHON_IMPLEMENTATION}-${PYTHON_VERSION} | ||
build: | ||
build: &base-build | ||
context: . | ||
dockerfile: ./Containerfile | ||
target: base | ||
args: | ||
- PYTHON_IMPLEMENTATION=${PYTHON_IMPLEMENTATION} | ||
- PYTHON_VERSION=${PYTHON_VERSION} | ||
|
@@ -139,6 +140,10 @@ volumes: | |
|
||
services: | ||
|
||
# Base service to allow building the image with `docker compose build base`. | ||
base: | ||
<<: *base | ||
|
||
# Services: Databases | ||
|
||
mariadb-db: | ||
|
@@ -285,6 +290,17 @@ services: | |
|
||
sqlite: | ||
<<: *base | ||
image: "django-docker-box:${PYTHON_IMPLEMENTATION}-${PYTHON_VERSION}\ | ||
-sqlite${SQLITE_VERSION}" | ||
pull_policy: never | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't push the image to Docker Hub. This ensures doing Could also try There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Ahh I see. I need to build the base first with Might be worth documenting. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Unfortunately the solution they propose doesn't work when the image that you depend upon is parametrized diff --git a/compose.yml b/compose.yml
index 301fb3e..e5d99f6 100644
--- a/compose.yml
+++ b/compose.yml
@@ -320,7 +320,9 @@ services:
- PYTHON_VERSION=${PYTHON_VERSION}
- SQLITE_VERSION=${SQLITE_VERSION}
- SQLITE_CFLAGS=${SQLITE_CFLAGS}
- additional_contexts: *additional-contexts
+ additional_contexts:
+ <<: *additional-contexts
+ "django-docker-box:${PYTHON_IMPLEMENTATION}-${PYTHON_VERSION}": "service:base"
depends_on:
<<: *depends-on-caches
environment: The only solution I could find was to use diff --git a/compose.yml b/compose.yml
index 301fb3e..10b4a03 100644
--- a/compose.yml
+++ b/compose.yml
@@ -142,6 +142,7 @@ services:
# Base service to allow building the image with `docker compose build base`.
base:
<<: *base
+ command: --help
# Services: Databases
@@ -323,6 +324,8 @@ services:
additional_contexts: *additional-contexts
depends_on:
<<: *depends-on-caches
+ base:
+ condition: service_completed_successfully
environment:
- DATABASE_ENGINE=django.db.backends.sqlite3 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 diff --git a/compose.yml b/compose.yml
index 301fb3e..9f761fb 100644
--- a/compose.yml
+++ b/compose.yml
@@ -294,7 +294,7 @@ services:
build:
context: .
dockerfile_inline: |
- FROM django-docker-box:${PYTHON_IMPLEMENTATION}-${PYTHON_VERSION}
+ FROM base
SHELL ["/bin/bash", "-o", "errexit", "-o", "nounset", "-o", "pipefail", "-o", "xtrace", "-c"]
# Only compile SQLite and set LD_PRELOAD if a version is specified.
RUN <<EOF
@@ -320,7 +320,9 @@ services:
- PYTHON_VERSION=${PYTHON_VERSION}
- SQLITE_VERSION=${SQLITE_VERSION}
- SQLITE_CFLAGS=${SQLITE_CFLAGS}
- additional_contexts: *additional-contexts
+ additional_contexts:
+ <<: *additional-contexts
+ base: service:base
depends_on:
<<: *depends-on-caches
environment: It results in
Which is unsurprising as even their minimal example fails to run (when fixing their duplicate services:
base:
build:
context: .
dockerfile_inline: |
FROM alpine
RUN ...
other: -- This was base wrongly set to base in their example
build:
context: .
dockerfile_inline: |
FROM base # image built for service base
RUN ...
additional_contexts:
base: service:base
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: diff --git a/Containerfile b/Containerfile
index 3e5cf8a..6d3fa95 100644
--- a/Containerfile
+++ b/Containerfile
@@ -1,5 +1,6 @@
# syntax=docker/dockerfile:1.12
+ARG BASE_IMAGE=pass
ARG PYTHON_IMPLEMENTATION=python
ARG PYTHON_VERSION=3.12
FROM ${PYTHON_IMPLEMENTATION}:${PYTHON_VERSION}-slim-bookworm AS base
@@ -65,7 +66,7 @@ VOLUME /django/source
WORKDIR /django/source/tests
ENTRYPOINT ["/django/entrypoint.bash"]
-FROM base AS sqlite
+FROM ${BASE_IMAGE} AS sqlite
ARG SQLITE_VERSION
ARG SQLITE_CFLAGS
SHELL ["/bin/bash", "-o", "errexit", "-o", "nounset", "-o", "pipefail", "-o", "xtrace", "-c"]
diff --git a/compose.yml b/compose.yml
index c18d32a..49a4157 100644
--- a/compose.yml
+++ b/compose.yml
@@ -7,6 +7,7 @@ x-base: &base
dockerfile: ./Containerfile
target: base
args:
+ - BASE_IMAGE=pass
- PYTHON_IMPLEMENTATION=${PYTHON_IMPLEMENTATION}
- PYTHON_VERSION=${PYTHON_VERSION}
additional_contexts:
@@ -297,6 +298,7 @@ services:
<<: *base-build
target: sqlite
args:
+ - BASE_IMAGE=django-docker-box:${PYTHON_IMPLEMENTATION}-${PYTHON_VERSION}
- PYTHON_IMPLEMENTATION=${PYTHON_IMPLEMENTATION}
- PYTHON_VERSION=${PYTHON_VERSION}
- SQLITE_VERSION=${SQLITE_VERSION} but I'm not sure if that'd be acceptable. The Also that approach would couple the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that current state of things with the multi-stage approach is fine even if it's no optimal so I'd stick to it for now. |
||
build: | ||
<<: *base-build | ||
target: sqlite | ||
args: | ||
- PYTHON_IMPLEMENTATION=${PYTHON_IMPLEMENTATION} | ||
- PYTHON_VERSION=${PYTHON_VERSION} | ||
- SQLITE_VERSION=${SQLITE_VERSION} | ||
- SQLITE_CFLAGS=${SQLITE_CFLAGS} | ||
depends_on: | ||
<<: *depends-on-caches | ||
environment: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,4 +14,5 @@ libpq-dev | |
libproj-dev | ||
libsqlite3-mod-spatialite | ||
pkg-config | ||
tcl-dev | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This requires re-building There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The distinct image approach solved the issue for me so I think we can stick to it without the optimization for now. Having it main for Django Con Europe sprints might be handy. |
||
unzip |
Uh oh!
There was an error while loading. Please reload this page.