-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Streamlined and added CI steps for docker compose #2207
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 6 commits
0b3585d
726c7b1
a134253
03e39ba
a7a672b
d1cb373
67514f5
68b0b20
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 |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# Copied from .gitignore | ||
__pycache__ | ||
data | ||
*.pyc | ||
*.db | ||
locale/*/LC_MESSAGES/django.mo | ||
*/locale/*/LC_MESSAGES/django.mo | ||
.sass-cache/ | ||
.coverage | ||
.direnv | ||
.envrc | ||
djangoproject/cache | ||
djangoproject/static/css/*.map | ||
djangoproject/static/css/*.css | ||
# Additional ignores for Docker | ||
.git/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,10 +25,24 @@ jobs: | |
- name: Checkout repository | ||
uses: actions/checkout@v4 | ||
|
||
- name: Test docker image build (local development) | ||
- if: matrix.req_file != 'tests.txt' | ||
name: Test docker image build (${{ matrix.req_file }}) | ||
uses: docker/build-push-action@v6 | ||
with: | ||
context: . | ||
push: false | ||
build-args: | | ||
REQ_FILE=requirements/${{ matrix.req_file }} | ||
|
||
- if: matrix.req_file == 'tests.txt' | ||
name: Set up Docker Compose | ||
uses: docker/setup-compose-action@v1 | ||
|
||
- if: matrix.req_file == 'tests.txt' | ||
name: Run the tests via Docker | ||
run: | | ||
docker compose build | ||
docker compose up -d db | ||
# Wait for Postgres to be ready | ||
docker compose exec db sh -c 'until pg_isready ; do sleep 1; done' | ||
tobiasmcnulty marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
tobiasmcnulty marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
docker compose run --rm web make ci | ||
Comment on lines
37
to
45
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. Does this affect the existing test workflow? It looks like we will be running tests in CI twice. 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.
That's correct, as a regression test for #2193 / assurance that the Longer term it might be nice to consolidate the settings so they're the same for local dev docker and non-docker, and run tests only in docker, but I don't mind running them twice in the interim. 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.
Wouldn't be enough to run 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 don't see the downside to running tests with both configurations temporarily, but I also don't mind removing 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. Any other feedback on this (either way), @django/django-website ? 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. My vote here would be to make Docker-based testing the main/only testing suite in CI in long term but I'm not sure it in this PR's scope. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,3 +30,10 @@ test: | |
|
||
watch-scss: | ||
watchmedo shell-command --patterns=*.scss --recursive --command="make compile-scss-debug" $(SCSS) | ||
|
||
reset-local-db: | ||
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 we can move 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. Personally I don't mind leaving What do others think? @django/django-website 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 creates problems when the contributor doesn't want to immedietaly apply the migrations. I think it's better that non of the data related operations are applied automatically, but I agree that contexts are different 👍🏻 |
||
python -m manage flush --no-input | ||
python -m manage loaddata dev_sites | ||
python -m manage loaddata doc_releases | ||
python -m manage loaddata dashboard_production_metrics | ||
python -m manage update_metrics |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#!/bin/sh | ||
|
||
PGPASSWORD=secret psql --username=code.djangoproject --dbname=code.djangoproject < /trac.sql |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,7 @@ | ||
#!/bin/sh | ||
|
||
python -m manage flush --no-input | ||
PGPASSWORD=secret psql --host db --port 5432 --username=code.djangoproject --dbname=code.djangoproject < tracdb/trac.sql | ||
python -m manage migrate | ||
make compile-scss # must come before collectstatic | ||
python -m manage collectstatic --no-input --clear | ||
python -m manage loaddata dev_sites | ||
python -m manage loaddata doc_releases | ||
# git config --global url."https://".insteadOf git:// | ||
# python -m manage update_docs | ||
python -m manage loaddata dashboard_production_metrics | ||
# python -m manage loaddata dashboard_example_data | ||
python -m manage update_metrics | ||
|
||
exec "$@" |
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.
For production, we just build the image, and then nothing else? 🤔
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 just building the image will be a weak test (and burn CI minutes). Since we are building the image, we might also want to check the startup, something like: