fix(review): PR #345 follow-ups — error hints and live-test cleanup - #346
fix(review): PR #345 follow-ups — error hints and live-test cleanup#346Zochory wants to merge 2 commits into
Conversation
Address PR #345 review follow-ups: - doctor database step now appends the closed, sanitized message from ensure_database_compatible so the shared remediation ("run `uv run python scripts/db_init.py`") reaches the operator surface; chained causes stay hidden. - supervisor preflight catch-all restores the actionable "verify FLEET_DATABASE_URL" hint alongside the sanitized message while keeping the dedicated DatabaseCompatibilityError remediation path. - live Lakebase DML round-trip uses engine.begin() transactional blocks (plus one PK-scoped transactional DELETE in teardown) so fixture-row lifecycle is atomic on the shared dev instance. - correct the two-phase flush comment in the session catalog: flush orders SQL statements within one transaction; it does not confer durability. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Narrow review follow-ups to #345 (Lakebase Postgres state), cut from
feat/lakebase-postgres-state:run_daytona_doctor's database step collapsed the sanitizedensure_database_compatibleexception into the fixed "Database connection or Alembic revision validation failed." string, so the shared remediation (runuv run python scripts/db_init.py``) never reached operators. The step message now appends the top-level sanitized message only for the closedDatabaseCompatibilityError/`DatabaseConnectionError` types; chained causes (paths, SQLAlchemy internals, secrets) stay hidden.raise SupervisorError(str(exc))) dropped the actionable "verify FLEET_DATABASE_URL" hint for connection failures. The hint is restored (f"{exc}; verify FLEET_DATABASE_URL") while the dedicatedDatabaseCompatibilityErrorpath keeps the remediation message untouched.test_lakebase_pooled_dml_round_tripused per-stepsession.commit()calls plus a secondDELETEinfinallyon a possibly re-pinged connection, risking an orphanedfleet_usersrow on the shared dev instance. Insert/delete now run inengine.begin()transactional blocks and teardown is a single PK-scoped transactionalDELETE; all SQL stays parameterized and assertions are unchanged.db.flush()does not commit. Reworded to describe dependency-ordered SQL statement ordering within one transaction.Test plan
uv run pytest tests/unit/backend/test_cli_supervisor.py tests/unit/backend/test_ensure_database_compatible.py tests/unit/backend/test_engine_pool_policy.py tests/unit/backend/test_daytona_diagnostics.py -q— 37 passedgit diff --check— cleanFLEET_LIVE=1) remains opt-in and unchanged in scopeMade with Cursor