feat(cli): Composite startup verbs for the flagsmith entrypoint#240
Conversation
Add serve, migrate, run-task-processor and migrate-and-serve verbs to the `flagsmith` CLI, mirroring the role sequencing Core API's run-docker.sh performs in shell but running in a single process so the Django app registry is initialised once instead of per step. The verbs stay generic: the databases to migrate or wait for, and any extra startup commands, are driven by settings the consuming app provides (FLAGSMITH_MIGRATE_DATABASES, FLAGSMITH_WAIT_FOR_MIGRATIONS_DATABASES, FLAGSMITH_STARTUP_COMMANDS). beep boop
Default --numthreads, --sleepintervalms, --graceperiodms and --queuepopsize from the TASK_PROCESSOR_* environment variables (falling back to the values run-docker.sh used), so the tuning survives the move to the `flagsmith` entrypoint. Adds environs to the task-processor extra, which now reads it. beep boop
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces composite startup commands to the flagsmith entrypoint to handle startup sequencing in a single process, and adds environment variable configuration for the task processor. The review feedback highlights a critical bug in how the legacy TASK_PROCESSOR_SLEEP_INTERVAL (seconds) is parsed as milliseconds, which can cause validation failures or high CPU usage. Additionally, the feedback suggests splitting startup commands with arguments using shlex.split to prevent execution failures, updating the unit tests accordingly, and clarifying the README documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #240 +/- ##
==========================================
+ Coverage 97.39% 97.56% +0.16%
==========================================
Files 104 108 +4
Lines 4613 4757 +144
==========================================
+ Hits 4493 4641 +148
+ Misses 120 116 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
FLAGSMITH_STARTUP_COMMANDS entries are command strings; passing an entry with arguments (e.g. "bootstrap --skip-fixtures") as a single argv element makes Django treat the whole string as an unknown subcommand. Shell-split each entry with shlex before dispatch. beep boop
khvn26
changed the title
matthewelwell
left a comment
matthewelwell
left a comment
There was a problem hiding this comment.
A couple of initial comments. Not quite ready to approve until I have answers, but not blocking so not worth a 'request changes' status.
|
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 PR adds composite startup verbs to the Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as flagsmith CLI
participant main as common.core.main
participant run as common.core.cli.run
participant waitfordb as waitfordb command
participant django as django_execute_from_command_line
CLI->>main: execute_from_command_line(argv)
main->>run: dispatch startup subcommand
run->>waitfordb: wait for DB / migrations when needed
run->>django: migrate / createcachetable / start api / task-processor
Related issues: None specified in the provided diff. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c54f0626-2423-4042-9557-c003ef34d47d
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
README.mdpyproject.tomlsrc/common/core/cli/run.pysrc/common/core/main.pysrc/task_processor/constants.pysrc/task_processor/utils.pytests/unit/common/core/test_main.pytests/unit/common/core/test_run.pytests/unit/task_processor/test_unit_task_processor_utils.py
khvn26
force-pushed
the
feat/composite-startup-verbs
branch
from
1729be2 to
10cdd9a
Compare
4 participants
Changes
Closes #239.
In this PR, we add the composite startup verbs to the
flagsmithCLI, fully compatible with Core API'srun-docker.sh.New
flagsmithverbs:servemigraterun-task-processormigrate-and-serveThey mirror
run-docker.sh's sequencing exactly, but run in a single process, so the Django app registry is initialised once instead of once permanage.pysubprocess. They stay generic — no Core-API specifics — driven by settings the consuming app provides:FLAGSMITH_MIGRATE_DATABASES(default["default"]) — databases themigratestep applies, in orderFLAGSMITH_WAIT_FOR_MIGRATIONS_DATABASES(default["default"]) — databasesrun-task-processorwaits onFLAGSMITH_STARTUP_COMMANDS(default[]) — commands run between migrate and serve inmigrate-and-serveSKIP_WAIT_FOR_DBis honoured in the verb layer. The server hand-off goes through the existingstartcommand via the management CLI path (notcall_command, which can't pass throughstart's gunicorn arguments alongside its subparsers).Separately, the task processor's worker tuning arguments (
--numthreads,--sleepintervalms,--graceperiodms,--queuepopsize) now default from theTASK_PROCESSOR_*environment variables, falling back to the valuesrun-docker.shused, so that tuning survives the move to the entrypoint.environsis added to thetask-processorextra, which now reads it.The README documents the new verbs and their settings and environment variables.
How did you test this code?
Unit tests cover each verb's exact call sequence, the settings-driven database lists, the bare-vs-targeted
migratesplit,SKIP_WAIT_FOR_DB, argument pass-through, dispatch routing, and the task processor env defaults, including theTASK_PROCESSOR_SLEEP_INTERVALfallback and precedence.