Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the repo’s legacy “remote devstack” integration and reorients local Docker usage to a lightweight, test-only container, with full runtime (app/worker/DB/etc.) now expected to be managed directly by the central devstack repository.
Changes:
- Deletes the repo-specific devstack provisioning script and removes/renames related Makefile targets.
- Replaces the existing multi-service
docker-compose.ymlwith a single, idle “test runner” container configured forenterprise_access.settings.test. - Updates project and agent documentation to reflect the new devstack-vs-local-tests split and new Docker usage patterns.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/README.md |
Updates troubleshooting guidance to assume the API server is run via devstack. |
provision-enterprise-access.sh |
Removes repo-local provisioning logic (now owned by devstack). |
docker-compose.yml |
Simplifies to a single test-only container (no DB/worker/etc.). |
README.rst |
Reframes setup docs: devstack for full runtime; local compose for tests/quality only. |
Makefile |
Removes old devstack integration targets; adds simplified dev.*/app-* docker-compose shortcuts. |
CLAUDE.md |
Updates Docker development notes to match the new model. |
.ralph/PROMPT.md |
Updates Docker development notes to match the new model. |
.claude/skills/unit-tests/SKILL.md |
Updates container interaction commands to docker compose patterns and adds troubleshooting. |
.claude/skills/quality-tests/SKILL.md |
Updates container interaction commands to docker compose patterns and adds troubleshooting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #104 +/- ##
=======================================
Coverage 84.28% 84.28%
=======================================
Files 144 144
Lines 12209 12209
Branches 1162 1162
=======================================
Hits 10290 10290
Misses 1598 1598
Partials 321 321 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pwnage101
force-pushed
the
pwnage101/sever-from-devstack
branch
from
28ec0bb to
74759b1
Compare
pwnage101
force-pushed
the
pwnage101/sever-from-devstack
branch
from
74759b1 to
fd75d9e
Compare
| $ make app-shell | ||
| # python ./manage.py migrate | ||
|
|
||
| Setting up openedx-events |
There was a problem hiding this comment.
This whole section should be safe to remove because last year I spent a bunch of time ensuring edx/devstack sufficiently launches enterprise-specific consumers automatically.
There was a problem hiding this comment.
Pull request overview
This PR removes enterprise-access’s repo-local “remote devstack” integration in favor of devstack-managed services, and repurposes this repo’s docker-compose.yml to provide only a lightweight container for running tests/quality checks locally.
Changes:
- Simplify
docker-compose.ymlto a single long-runningappcontainer intended for executing tests/quality commands only. - Remove the repo-local provisioning script and devstack-oriented Makefile targets; replace them with docker-compose v2 (
docker compose) shortcuts for test/quality workflows. - Update docs/prompts/skills to reflect devstack as the source of truth for running the full service and to use
docker compose exec ...for local test execution.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/README.md | Updates troubleshooting guidance to assume the API runs via devstack. |
| provision-enterprise-access.sh | Deletes legacy provisioning workflow now expected to live in devstack. |
| docs/customer-billing/billing-management-api.rst | Updates test execution command to use docker compose exec. |
| docker-compose.yml | Replaces multi-service devstack-style compose with a single test-only container. |
| README.rst | Reframes setup instructions: full runtime via devstack; local compose only for tests/quality. |
| Makefile | Removes devstack lifecycle targets; adds compose v2 shortcuts for local test container usage. |
| CLAUDE.md | Updates “Docker Development” notes to reflect devstack ownership of full runtime. |
| .ralph/PROMPT.md | Same as above for Ralph prompt guidance. |
| .claude/skills/unit-tests/SKILL.md | Updates container discovery and test commands to docker compose + adds troubleshooting. |
| .claude/skills/quality-tests/SKILL.md | Updates quality commands to docker compose + adds troubleshooting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pwnage101
force-pushed
the
pwnage101/sever-from-devstack
branch
from
fd75d9e to
57cb236
Compare
pwnage101
force-pushed
the
pwnage101/sever-from-devstack
branch
from
57cb236 to
ce10452
Compare
There was a problem hiding this comment.
Pull request overview
This PR removes this IDA’s “remote devstack” integration and reorients the repo’s Docker/Makefile workflow to support only a lightweight local test/quality container, with full runtime orchestration handled directly by the central devstack repository.
Changes:
- Replace the repo’s
docker-compose.ymlwith a single “test-only”appcontainer (no DB/worker/etc.). - Remove local devstack provisioning/start targets and update Makefile shortcuts to use
docker compose. - Update docs/skill prompts to reflect the new devstack-first runtime model and the test-container workflow.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/README.md | Updates troubleshooting guidance to reference devstack instead of repo-local make dev.up. |
| provision-enterprise-access.sh | Removes repo-local provisioning script for devstack integration. |
| docs/customer-billing/billing-management-api.rst | Updates test execution instructions to use docker compose exec app. |
| docker-compose.yml | Simplifies compose to a single, test-only app container. |
| README.rst | Updates setup instructions to point runtime usage to devstack and local usage to test/quality container. |
| Makefile | Removes old devstack integration targets and adds docker compose-based test-container shortcuts + aliases. |
| CLAUDE.md | Updates Docker development notes to reflect devstack-first runtime model. |
| .ralph/PROMPT.md | Updates Docker development notes to reflect devstack-first runtime model. |
| .claude/skills/unit-tests/SKILL.md | Updates test-running commands to use docker compose and adds troubleshooting notes. |
| .claude/skills/quality-tests/SKILL.md | Updates quality-running commands to use docker compose and adds troubleshooting notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pwnage101
force-pushed
the
pwnage101/sever-from-devstack
branch
from
ce10452 to
e890857
Compare
pwnage101
force-pushed
the
pwnage101/sever-from-devstack
branch
from
e890857 to
2c41b84
Compare
pwnage101
force-pushed
the
pwnage101/sever-from-devstack
branch
from
2c41b84 to
f610b7e
Compare
There was a problem hiding this comment.
Pull request overview
This PR completes the “devstack integration removal” for enterprise-access by shifting local Docker usage in this repo to test/quality execution only, and pointing full runtime development (app/worker/DB/etc.) to be managed directly by the central devstack repository.
Changes:
- Simplifies
docker-compose.ymlto a single long-livedappcontainer intended only for running tests/quality commands. - Reworks Makefile Docker targets to use
docker composeand to manage only the test/quality container lifecycle. - Updates documentation and internal “agent/skill” guidance to reflect the new devstack-first runtime model and the new test-container commands.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/README.md | Updates troubleshooting guidance to assume the API server comes from devstack rather than repo-local compose. |
| provision-enterprise-access.sh | Removes legacy repo-local provisioning script for remote-devstack model. |
| docs/customer-billing/billing-management-api.rst | Updates test execution command to use docker compose exec app rather than docker exec with the old container name. |
| docker-compose.yml | Replaces the full multi-service devstack-style compose setup with a minimal test-only container. |
| README.rst | Reframes setup docs around devstack for full runtime and local compose for tests/quality only. |
| Makefile | Removes devstack-integration targets and adds docker compose targets for managing the test/quality container. |
| CLAUDE.md | Updates Docker development notes to match the new devstack-first model. |
| .ralph/PROMPT.md | Updates Docker development notes to match the new devstack-first model. |
| .claude/skills/unit-tests/SKILL.md | Updates commands to use docker compose, and adds colima troubleshooting guidance. |
| .claude/skills/quality-tests/SKILL.md | Updates commands to use docker compose, and adds colima troubleshooting guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pwnage101
force-pushed
the
pwnage101/sever-from-devstack
branch
from
f610b7e to
ebd7514
Compare
|
Created this runbook to assist with migration: https://2u-internal.atlassian.net/wiki/x/J4CXy |
| [pytest] | ||
| DJANGO_SETTINGS_MODULE = enterprise_access.settings.test | ||
| addopts = --cov enterprise_access --cov-report term-missing --cov-report xml | ||
| norecursedirs = .* docs requirements site-packages scripts | ||
|
|
There was a problem hiding this comment.
It's superseded by pytest.ini for better consistency with the other repositories. Not necessarily the best solution, but seemed logical given pytest.local.ini is sibling so it's easy to find both.
Transition away from the "remote devstack" model where IDAs implemented their own devstack integration. This is no longer necessary because all IDAs should now be directly integrated with devstack by adding make targets, provision scripts, and docker-compose services DIRECTLY within edx/devstack. - Overhaul docker-compose.yml to completely change the purpose of the `app` container to be exclusively for running tests. - Update a bunch of other files to reflect the new purpose of the testing-only `app` container. Doing this first in enterprise-access to serve as a model we can test and iron out before exporting to the rest of enterprise IDAs.
pwnage101
force-pushed
the
pwnage101/sever-from-devstack
branch
from
ebd7514 to
1a6f7e0
Compare
There was a problem hiding this comment.
Pull request overview
This PR removes enterprise-access’s legacy “remote devstack” integration and re-scopes this repo’s local Docker usage to a lightweight, test/quality-only container, with full runtime managed directly by edx/devstack. No critical issues found.
Changes:
- Simplifies
docker-compose.ymlto a single long-running container intended for executing tests/quality checks. - Removes devstack provisioning/start targets and scripts, updating documentation and internal agent/skill docs accordingly.
- Updates Docker command examples to use
docker composeand aligns settings for the test container.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tox.ini |
Removes embedded [pytest] config (now covered by pytest.ini). |
scripts/README.md |
Updates troubleshooting guidance to reference devstack instead of local make dev.up. |
README.rst |
Replaces devstack-workspace setup docs with pointers to devstack + local test container workflow. |
provision-enterprise-access.sh |
Deletes legacy provisioning script used for remote devstack integration. |
Makefile |
Removes devstack orchestration targets and adds docker compose-based shortcuts for the test container. |
docs/customer-billing/billing-management-api.rst |
Updates test execution command to docker compose exec. |
docker-compose.yml |
Replaces multi-service dev runtime with a single “test runner” container. |
CLAUDE.md |
Updates Docker development notes to reflect devstack ownership + test-only compose usage. |
.ralph/PROMPT.md |
Same: updates Docker development guidance for Ralph. |
.claude/skills/unit-tests/SKILL.md |
Updates container commands to docker compose, adds colima troubleshooting. |
.claude/skills/quality-tests/SKILL.md |
Updates container commands to docker compose, adds colima troubleshooting. |
| # Docker in this repo is only supported for running tests locally. | ||
| services: |
There was a problem hiding this comment.
The header comment says Docker in this repo is only supported for running tests, but the Makefile/README/CLAUDE docs all describe this compose setup as supporting both tests and quality checks. Suggest updating this comment to match the intended scope so contributors don’t assume make quality/make validate aren’t supported in this container.
| app-restart-devserver: # restart just the app Django dev server | ||
| docker-compose exec app bash -c 'kill $$(ps aux | egrep "manage.py ?\w* runserver" | egrep -v "while|grep" | awk "{print \$$2}")' | ||
| dev.makemigrations: ## Create migrations via the app container | ||
| docker compose exec app python manage.py makemigrations |
There was a problem hiding this comment.
dev.makemigrations runs manage.py makemigrations inside the docker-compose container, but the compose service sets DJANGO_SETTINGS_MODULE=enterprise_access.settings.test. That test settings module adds the enterprise_access.apps.workflow.tests app (with its own models/migrations), which can cause makemigrations to detect or generate migrations for test-only models. Consider overriding the settings module for this target (e.g., use the local settings module) so migration generation reflects the real app configuration.
| docker compose exec app python manage.py makemigrations | |
| docker compose exec -e DJANGO_SETTINGS_MODULE=enterprise_access.settings.local app python manage.py makemigrations |
Note: This is a continuation of something I started in edx-enterprise: openedx/edx-enterprise#2538 but this is the first IDA to sever from devstack.
Transition away from the "remote devstack" model where IDAs implemented their own devstack integration. This is no longer necessary because all IDAs should now be directly integrated with devstack by adding make targets, provision scripts, and docker-compose services DIRECTLY within edx/devstack.
appcontainer to be exclusively for running tests.appcontainer.Doing this first in enterprise-access to serve as a model we can test and iron out before exporting to the rest of enterprise IDAs.