forked from orf/django-docker-box
-
Notifications
You must be signed in to change notification settings - Fork 38
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
Refactored to fix multiple issues. #48
Open
ngnpope
wants to merge
48
commits into
django:main
Choose a base branch
from
ngnpope:refactor
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `django-box` repository was archived over 5 years ago and many people coming to this now will have no idea what Vagrant or VirtualBox even are.
Travis CI was used when GitHub Actions was still very new and had limited support. Since then, Travis CI became less tolerant of open source projects and runtimes increased significantly. GitHub Actions provides much more flexibility and can be looked into as a replacement for this matrix of tests.
This looks incomplete and was likely considered as an alternative to Travis CI at some point. We should look into GitHub Actions as a replacement for running a matrix of tests.
Ownership of the repository for images on Docker Hub is likely in the hands of the original author rather than the Django project itself. In addition, we should probably look at making use of the GitHub Container Repository instead which will be easier to manage and control.
We will be able to support Oracle again now that they have started publishing container images themselves via their own repository. It's easier to remove this broken support and start again, however, as - aside from using a very old version of Oracle Database - this approach required many hacks.
This raises a warning when running `docker compose`, so remove it.
Django now bundles test Maxmind GeoIP and GeoLite databases which means that this setting - and a manual download of these databases - is no longer required.
Instead of `docker-compose.yml`, rename to `compose.yml` as supported by the Compose Specification. Also rename to `Containerfile` instead of using `Dockerfile` to be more tool-agnostic. It would be good to also support Podman if and when it supports all of the features we require.
Most settings are exactly the same, so by dynamically generating the `DATABASES` setting we can ensure greater consistency and reduce the maintenance required. The `TEST_RUNNER` and `TEST_OUTPUT_DIR` settings have been hidden behind an `XUNIT` environment variable. These are typically only useful in CI, specifically on Jenkins, suppress some output, and do not support various features such as debugging SQL output and test durations.
All other database use the full name, so do the same for PostgreSQL.
Grouped services and commands by type and ordered alphabetically within each section.
The MySQL native password authentication plugin is obsolete.
This allow us to apply configuration consistently and reduce duplication.
And also ensure that they restart unless explicitly stopped.
This integrates better with compose commands with feedback on service health. Note that this doesn't yet introduce healthchecks for selenium nodes as we'll be switching over to Selenium Grid in a future commit which works slightly differently and the current setup doesn't seem to work correctly.
Prior to this MySQL and MariaDB were creating a "django" user, but were using "root" because the user didn't have the appropriate permissions. Fixing that brings the credentials used in line with PostgreSQL.
Also reintroduces healthchecks.
This disables various features in databases and attempts to reduce disk writes such that testing is as fast as possible.
This is now using the images provided by Oracle in their own container registry.
Ensures that tests can be run for the cache backend for Redis.
There's not much reason why these need to be disabled.
Use the top-level `/django` folder as a place to copy files and mount volumes. The Django source code will now be mounted to `/django/source` and any generated output, e.g. coverage, will be output to a mounted volume at `/django/output`. It is also no longer necessary to specify the `DJANGO_PATH` environment variable if you have your Django respository checked out alongside this one, e.g. ``` ~/Sources ├── django └── django-docker-box ``` If you keep your source elsewhere, then the default of `../django` can still be overridden using `DJANGO_PATH`. Currently the Django sources are mounted read-write. This is due to various tools that are currently configured to write output into the source tree, e.g. coverage. In addition, some tests may write files to the source tree instead of a temporary location. Hopefully we can gradually address these issues and then move to mounting the source tree read-only instead. This commit also sets up the Django source tree as an additional context instead of the default context. This will allow including files from this repository during the image build but also sets up sourcing requirements files from the source tree which will be crucial to improving the container file.
It's much easier to maintain a list of packages to install in a separate file. We can also get the packages required to build the documentation from the requirements file in the Django source tree.
Use modern syntax including heredocs, cache mounts, etc. Also merge the requirements files to reduce to a single install command.
This will be expanded in future for enabling coverage, etc.
Also update some packages to install the dev packages. These will install the correct runtime libraries and keeps this maintainable - the version numbers in the package names can change with each distro update, so using the dev package avoids manual updates.
Ensure that we'll get colored output when running in GitHub Actions.
Using `pre-commit` makes it easier to run all linters with the versions pinned by Django. Also add `black` as a dependency so that migrations which are created when using these images will be formatted.
Recently this was updated to use `utf8mb4` for Django 5.2
Hopefully we'll reinstate this in the future, but make use of GitHub Container Registry. It'll be easier to maintain and we should be able to publish all versions on a regular basis from a GitHub Actions workflow.
This isn't necessary - we can just use the current directory as there is a lot of other stuff already doing that. Using `${PWD}` makes using this on Windows more awkward as this environment variable is undefined by default.
Add much more guidance on using the tools within this repository.
That's quite the 🎄 gift indeed Nick!!! I should be able to give it a run during the holidays but assuming this is based on your work in the Oracle issue branch I can confirm that I've been using with success for Oracle, MySQL, and Postgres for about 6 months now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As it's nearly Christmas 🎄 I thought I'd share this gift 🎁
Tagging those who have expressed interest: @charettes @sarahboyce
Includes the following changes (in no particular order):
PIP_CACHE_VOLUME
from.env
.gitignore
docker_push.sh
scriptpre-commit
,hadolint
,ruff
,yamllint
, andzizmor
for lintingDJANGO_PATH
to../django
, i.e. assume adjacent checkouts${PWD}
which is not available on WindowsDockerfile
to use modern syntax, including:RUN
to avoid backslash escaping--mount=type=cache,...
withRUN
to improve build speedapt-get
andpip
installation to be more optimal:uv
for installation of Python dependenciesset -o errexit -o nounset -o pipefail -o xtrace
forRUN
Dockerfile
for use in CIdocker-compose.yml
to use modern features:version
propertyadditional_contexts
to source files from multiple placesdeploy: mode: global
for databases, caches, etc.healthcheck
in place ofwait-for-it.sh
Dockerfile
toContainerfile
docker-compose.yml
tocompose.yml
Fixes #4, closes #5, resolves #8, fixes #42, closes #47.