feat(server): bundle migrations in wheel and add agent-control-migrate#209
feat(server): bundle migrations in wheel and add agent-control-migrate#209abhinav-galileo wants to merge 4 commits intomainfrom
Conversation
Ships server/alembic/ and alembic.ini inside the installed package via hatch force-include, and exposes a small console script that runs 'alembic upgrade head' against the bundled config. Lets non-Docker consumers run migrations from a wheel install without vendoring.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Adds success-path coverage for upgrade/downgrade/current/history/heads via monkeypatch-and-assert against alembic.command, plus a build-time guard that the source paths hatch force-include reads from still exist in-tree (catches accidental wheel regressions).
Drops the implicit '-1' default for 'agent-control-migrate downgrade'. Downgrade is destructive; an operational CLI should not roll back the schema on a typo or incomplete command. Users must now pass an explicit revision (e.g. 'downgrade -1' or 'downgrade <rev>').
| ``upgrade``, ``downgrade``, ``current``, ``history``, ``heads``. | ||
| """ | ||
| args = list(argv) if argv is not None else sys.argv[1:] | ||
| if not args: |
There was a problem hiding this comment.
P1: Unhandled exceptions from Alembic commands propagate as tracebacks
In migrate.py: lines42-65 none of the command.*() calls are wrapped in try/except. If the DB is unreachable (OperationalError), a migration is in conflict (CommandError), or the lock can't be acquired, the exception propagates uncaught. The raise SystemExit(main()) entry point never executes, so the caller gets a traceback + exit code 1.
For an operational deploy CLI this is fragile — deploy pipelines typically expect a clean error message and a predictable non-zero exit code. Recommend wrapping each command dispatch in:
try:
command.upgrade(cfg, ...)
except Exception as exc:
print(f"agent-control-migrate: {exc}", file=sys.stderr)
return 1
There was a problem hiding this comment.
Addressed in f21c959: alembic dispatch is now wrapped in try/except Exception; failures print agent-control-migrate: <reason> to stderr and return rc=1 instead of leaking a traceback. Covered by test_main_returns_nonzero_for_command_errors.
| elif op == "heads": | ||
| command.heads(cfg) | ||
| else: | ||
| print(f"agent-control-migrate: unknown command '{op}'", file=sys.stderr) |
There was a problem hiding this comment.
--help/-h prints "unknown command" instead of usage
agent-control-migrate --help falls into the else branch and exits with code 2. Operational teams will hit this immediately. A simple check for op in ("-h", "--help") printing the supported-commands string would fix it.
There was a problem hiding this comment.
Addressed in f21c959: rewrote the parser as argparse with subparsers, so --help/-h prints usage natively (rc=0). Covered by test_main_help_prints_usage.
| """ | ||
| args = list(argv) if argv is not None else sys.argv[1:] | ||
| if not args: | ||
| args = ["upgrade", "head"] |
There was a problem hiding this comment.
Extra arguments to upgrade/downgrade are silently dropped:
command.upgrade(cfg, rest[0] if rest else "head"). Running agent-control-migrate upgrade rev1 extra silently ignores extra. A typo like agent-control-migrate upgrade -1 (intended as a downgrade) passes "-1" to upgrade silently. Consider rejecting len(rest) > 1 with an error.
There was a problem hiding this comment.
Addressed in f21c959: argparse now rejects extra positional args with a usage error. Covered by test_main_rejects_extra_positional_args. --sql for upgrade/downgrade is also wired through to alembic.command.upgrade(..., sql=True).
| @@ -0,0 +1,115 @@ | |||
| """Unit tests for the bundled-migrations entry point. | |||
There was a problem hiding this comment.
Test gap: bare upgrade keyword with no revision is untested
covers main([]) (default → upgrade head) and main(["upgrade", "abc123"]) (explicit revision), but not main(["upgrade"]) (bare keyword → should also call upgrade head). That path is exercised by rest[0] if rest else "head" but has no dedicated test case.
There was a problem hiding this comment.
Addressed in f21c959: added test_main_bare_upgrade_runs_upgrade_head covering main(["upgrade"]) → upgrade head.
2 participants
Summary
Ship Alembic migration scripts and
alembic.iniinside the installed wheel via hatchforce-include, and expose a smallagent-control-migrateconsole script that runsalembic upgrade headagainst the bundled config.Lets non-Docker consumers (e.g. wheel-based deployments) run migrations from a vanilla install without vendoring migration files.
Scope
agent-control-migrateconsole script (subcommands:upgrade,downgrade,current,history,heads).agent_control_server/_alembic/andagent_control_server/_alembic.ini(private, underscore-prefixed).alembic upgrade headfromserver/still works.Risk and Rollout
Testing
server/unit_tests/test_migrate.py).agent_control_server/_alembic/versions/*.pyandagent_control_server/_alembic.ini.ruff check(server and root configs).agent-control-migrateagainst a real database.Checklist