feat: Adopt the flagsmith entrypoint#7901
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
github-actions
Bot
added
api
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7901 +/- ##
=======================================
Coverage 98.63% 98.63%
=======================================
Files 1497 1497
Lines 59189 59189
=======================================
Hits 58383 58383
Misses 806 806 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
khvn26
requested review from
emyller and
germangarces
and removed request for
a team
github-actions
Bot
added
feature
Docker builds report
|
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise
Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise
Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change adds Flagsmith migration orchestration settings in Django, switches container startup to invoke Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Dockerfile
participant run-docker.sh
participant flagsmith
Dockerfile->>flagsmith: ENTRYPOINT flagsmith
run-docker.sh->>flagsmith: exec flagsmith "$@"
Related issues: None specified in the provided context. Related PRs: None specified in the provided context. Suggested labels: infrastructure, dependencies, settings Suggested reviewers: None specified in the provided context. Comment |
Replace the bespoke startup sequencing in run-docker.sh with the composite verbs provided by the flagsmith-common `flagsmith` entrypoint: - run-docker.sh becomes a thin `exec flagsmith "$@"` shim, kept for callers that reference the script path or its verbs directly. - The image ENTRYPOINT is now `flagsmith`; the APPLICATION_LOGGERS default the shell script used to set moves to a Dockerfile ENV. - Startup verbs are driven by new settings built from the configured databases (FLAGSMITH_MIGRATE_DATABASES, FLAGSMITH_WAIT_FOR_MIGRATIONS_DATABASES, FLAGSMITH_STARTUP_COMMANDS). - The task processor task definition starts via `start task-processor` and no longer migrates on every task; migrations run once per deploy via the migration task. startPeriod drops from 120s to 30s for the faster boot. flagsmith-common is temporarily sourced from its branch until the verbs are released; revert to the PyPI release before merge (see pyproject TODO). beep boop
Populate FLAGSMITH_MIGRATE_DATABASES and FLAGSMITH_WAIT_FOR_MIGRATIONS_DATABASES as each database alias is configured, instead of reconstructing them from a _configured_databases snapshot after the fact. Bump flagsmith-common to the 3.11.0 release, which ships the composite startup verbs, and drop the temporary git source. Remove the now-redundant prose comments from run-docker.sh and the settings. beep boop
khvn26
force-pushed
the
feat/adopt-flagsmith-entrypoint
branch
from
6669808 to
5ddf8e8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 31a303dc-1e5b-466f-8795-24835cf1cb65
⛔ Files ignored due to path filters (1)
api/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Dockerfileapi/app/settings/common.pyapi/pyproject.tomlapi/scripts/run-docker.shinfrastructure/aws/production/ecs-task-definition-task-processor.jsoninfrastructure/aws/staging/ecs-task-definition-task-processor.json
github-actions
Bot
added
feature
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/app/settings/common.py (1)
193-201: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSeed
defaultin the fallback database-config path Theif/elifalready avoids a double append, but when neitherDATABASE_URLnor the legacyDJANGO_DB_NAMEvars are set, both Flagsmith lists stay empty. That leavesdefaultout offlagsmith start, so the primary database is neither waited on nor migrated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d1625745-de7f-4a3d-9920-858d7c6d8724
⛔ Files ignored due to path filters (1)
api/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Dockerfileapi/app/settings/common.pyapi/pyproject.tomlapi/scripts/run-docker.shinfrastructure/aws/production/ecs-task-definition-task-processor.jsoninfrastructure/aws/staging/ecs-task-definition-task-processor.json
…h entrypoint The flagsmith-common `serve` verb runs a bare `flagsmith start api`, whose gunicorn defaults are GUNICORN_WORKERS=1 and GUNICORN_THREADS=1. The run-docker.sh serve() it replaces defaulted to 3 workers and 2 threads, so image consumers that do not set these (self-hosted deployments and the E2E suite) drop from six concurrent request handlers to one. Restore the previous defaults as image ENV, alongside APPLICATION_LOGGERS. SaaS is unaffected — its task definitions set both explicitly. beep boop
github-actions
Bot
added
infrastructure
feature
3 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #7891.
Adopts the
flagsmithCLI entrypoint (from flagsmith-common) for container startup, replacing the bespoke sequencing inapi/scripts/run-docker.sh.run-docker.shis reduced to a backwards-compatible shim (exec flagsmith "$@"), so anything invoking the script path or its verbs keeps working.ENTRYPOINTis nowflagsmith;APPLICATION_LOGGERS(previously defaulted in the shell script) moves to a DockerfileENVso logging is unchanged.app/settings/common.pyfrom the databases actually configured.start task-processor.startPeriodraised to 120s in fix(infra): Task processor tasks killed during startup by health check #7887 is lowered to 30s in both the prod and staging task-processor definitions, which comfortably covers the new single-boot startup.How did you test this code?
End-to-end against a local environment, exercising each verb via the
flagsmithentrypoint:flagsmith migrate— waits for the DB, migrates the configured databases, creates the cache table.flagsmith serve— waits for the DB, starts the API;/health/livenessserved on:8000.flagsmith start task-processor— the new task-definition command; binds:8000and starts the worker threads.flagsmith migrate-and-serve— runs migrate, thenbootstrap(viaFLAGSMITH_STARTUP_COMMANDS), then serves.