fix(workflows): gateway DDL + billing perm-error + tools tables + Lakebase smoke check - #20
Merged
Merged
Conversation
Phase 6 of `02_sync_to_lakebase.py` ran all gateway DDLs in a single transaction, with the ALTER for `gateway_usage_hourly` issued *before* its CREATE TABLE. The ALTER raises `UndefinedTable`; psycopg2 marks the whole transaction aborted, and the per-statement try/except swallows the warning while the trailing CREATE silently no-ops. Result on a fresh database: the table never gets created, the discovery job reports SUCCESS, and the deployed app's Gateway page logs a stream of `UndefinedTable: relation "gateway_usage_daily" does not exist`. Reorder DDLs so all CREATE TABLEs run before any CREATE INDEX/ALTER, and run each DDL in its own commit/rollback scope so a single failure cannot poison sibling DDLs. Verified on the Ecolab sandbox: post-fix discovery run materialises both `gateway_usage_daily` (21k rows) and `gateway_usage_hourly` (2.6k rows) in Lakebase, app errors stop. Also adds in-workspace SP-grant fallback (helper notebook + shell runner) for environments where the workspace blocks public Lakebase access from the laptop, and a docs/rca/ entry capturing this and the other deployment-blocking issues we hit during the Ecolab sandbox onboarding. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A new `smoke_check_lakebase` workflow task asserts that every Lakebase table the dashboard reads from exists with the expected row content. REQUIRED tables must exist with >=1 row; EXPECTED must exist (0 rows allowed, logged WARN); OPTIONAL (app-managed) existence is not asserted. Failure raises directly (not via dbutils.notebook.exit) so the workflow result_state flips to FAILED and CI can gate on it. Wired as `smoke_check_lakebase` in databricks.yml, depending on `sync_to_lakebase`. Split out from #18; CI/CD deploy pattern is in a separate PR. Co-authored-by: Isaac
The _execute_sql helper in 09_discover_billing.py returned [] on any non-SUCCEEDED state and only printed the error. When a-hakketh lacked SELECT on system.billing.list_prices, the three queries that JOIN list_prices (serving / product / user_cost) silently produced 0 rows; the task still reported result=SUCCESS and the cost-related dashboard sections rendered empty while token usage worked. Root cause was a missing schema-level grant — fixed in the workspace with GRANT SELECT ON SCHEMA system.billing. This commit makes the code fail loud instead of silently zero on the next time it happens: the helper now raises RuntimeError carrying the SQLSTATE and message. Verified: post-grant + post-fix discovery run produced 15,040 serving / 9,806 product / 4,472 user_cost rows, all syncing 1:1 to Lakebase. Captured as item 5 in the deployment findings RCA. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…rrors
The Tools page (Overview / MCP Servers / UC Functions / Usage) rendered
empty in the deployed app with /api/v1/tools/overview returning 500
"relation tool_registry does not exist". Three layered failures masked
each other:
1. tool_registry was never created. App startup runs ensure_tools_tables
in a daemon thread under t.join(timeout=120); if the thread crashed,
timed out, or hit Lakebase auth pressure, the table never landed and
the app booted regardless.
2. ensure_tools_tables swallowed real DDL failures with logger.warning,
indistinguishable from "already exists" cases.
3. refresh_tools wrapped its entire body in try/except and POST /tools/sync
returned 200 unconditionally, hiding the broken state from operators.
Same pattern applied to request_logs (created lazily by the audit
middleware, also failed silently if app SP lacked DDL).
Fix:
- workflows/02_sync_to_lakebase Phase 7 now creates tool_registry and
request_logs from the workflow run-as identity (databricks_superuser),
matching how every other Lakebase table in this stack is provisioned.
The app no longer needs DDL privileges to function.
- tools_service.refresh_tools self-heals by calling ensure_tools_tables
first, and switches catch-all to logger.exception for full tracebacks.
- workflows/10_smoke_check_lakebase asserts tool_registry and
request_logs in the EXPECTED bucket — missing table fails the
workflow, 0 rows is a WARN (legitimate before user activity).
Also adds RCA item 6 to docs/rca/2026-06-06-deploy-non-trivial-fixes.md.
Verified end-to-end on Ecolab sandbox (run 881046971716582):
overview now returns {total_tools:3, mcp_servers:3, uc_functions:0,
managed_count:3}; the three system-managed MCP connections render in
the UI. Empty UC Functions / Usage tabs are legitimate (no agent UC
functions or trace tool spans yet).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…verwrite job params While verifying the item-6 fix end-to-end, a `databricks bundle deploy --target dev` without explicit --var flags overwrote the running job's working catalog/warehouse_id parameters with the literal placeholder strings (`<your-catalog>`, `<your-warehouse-id>`) defined as defaults in workflows/databricks.yml's dev target. Every subsequent task failed with a SQL parse error. Captured the footgun, the recovery commands (the full --var= invocation that restores the working job), and proposed durable fixes (sentinel defaults that fail loud, or a gitignored .databricks-bundle.<target>.local.yml overlay). Verified clean re-run on Ecolab sandbox: 11/11 tasks SUCCESS, smoke check passed 12/12 REQUIRED tables OK, tool_registry and request_logs present and asserted in the EXPECTED bucket. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Closed
11 tasks
…Step 8) The billing fail-loud fix in this PR turns a missing SELECT on system.billing.list_prices into a hard discovery-task failure (SQLSTATE 42501) instead of silent 0-row cost dashboards. Document the required schema-level grants for the workflow run-as identity in Step 8 (grant the whole system.billing / system.serving schemas so a future table addition can't reintroduce the gap), and make the step non-optional. Closes the installation docs TODO tracked in the deploy RCA for item 5. Co-authored-by: Isaac
The CI/CD deployment-pattern ADR ships in the separate CI/CD PR, so a relative markdown link to it is dead on main until that PR merges. Reference the path as plain text instead, leaving #20 with no cross-PR loose ends. Co-authored-by: Isaac
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part (a) of splitting #18 (from @thijs-hakkenberg / Ecolab sandbox onboarding) into two focused PRs. This PR carries the three bug fixes + the Lakebase smoke check; the generic CI/CD pipeline lives in its companion PR.
All commits are rebased onto current
main, so #17's workload-classification work (classify_modelstask,workload_classcolumns) is preserved — the original #18 branch predated it.1. Gateway DDL transaction fix (
02_sync_to_lakebase.pyPhase 6)On a fresh DB, Phase 6 issued an
ALTER TABLE gateway_usage_hourlybefore the matchingCREATE TABLE. psycopg2 marks the whole transaction aborted on the first error, so the trailingCREATE TABLEs silently no-op'd — leaving the DB with neithergateway_usage_dailynorgateway_usage_hourly, while the discovery job still reportedSUCCESS. The deployed app then logged an unbounded stream ofUndefinedTableerrors and rendered the Gateway page empty.Fix: reorder so all
CREATE TABLEs precedeCREATE INDEX/ALTER, and run one transaction per DDL (per-statementcommit(),rollback()-and-continue on failure) so a first-runALTERcan't poison sibling DDLs. Verified: both tables land with rows (21,303 / 2,606), no moreUndefinedTable.2. Surface SQL permission errors in
09_discover_billing.py_execute_sqlreturned[]on any non-SUCCEEDEDstate — includingINSUFFICIENT_PERMISSIONS. A missingSELECT ON system.billing.list_pricesgrant silently produced 0-row cost dashboards (Cost Overview / Endpoint Costs / All Products) for weeks while token usage worked, and discovery still reportedSUCCESS.Fix: the helper now
raises with the SQLSTATE + message, so a permission gap fails the discovery task immediately and actionably. (The production fix is a one-lineGRANT, documented in the RCA — environmental, not code.)3. App-managed table DDL moved into the workflow (
tools_service.py+02_syncPhase 7)tool_registry/request_logswere created lazily by the app's startup daemon thread (join(timeout=120)). Under cold-start / Lakebase auth pressure the tables never landed;/api/v1/tools/overviewreturned 500, the Tools page rendered empty, and/api/v1/tools/syncreturned HTTP 200 regardless.Fix: new Phase 7 in
02_sync_to_lakebase.pycreates these tables from the workflow run-as (databricks_superuser) — same pattern as the other 20+ tables, so the app needs only DML.refresh_tools()self-heals (callsensure_tools_tables()first) and switches the catch-all fromlogger.warningtologger.exceptionso tracebacks land in app logs.4. Lakebase smoke check (
workflows/10_smoke_check_lakebase.py)A new
smoke_check_lakebasetask asserts every Lakebase table the dashboard reads from exists with the expected content — the regression gate the bugs above would have caught. Tables are bucketedREQUIRED(exist + ≥1 row),EXPECTED(exist, 0 rows → WARN),OPTIONAL(app-managed, not asserted). It raises directly (not viadbutils.notebook.exit(), which would mask the failure) soresult_stateflips toFAILEDfor CI to gate on, and inlines the per-table breakdown into the exception message (sincejobs/get-run-outputonly round-tripserror/error_trace). Wired indatabricks.yml, depending onsync_to_lakebase.On the Ecolab sandbox the first run surfaced the two billing tables that fix #2 then root-caused.
Plus (from the same onboarding)
grant_sp_lakebase_notebook.py+run_grant_sp_lakebase_job.sh): runs the Lakebase grant as a one-shot serverless Job for workspaces where public Lakebase ingress is blocked.deploy.shpoints to it.docs/rca/2026-06-06-deploy-non-trivial-fixes.md): deployment-blocking issues from the onboarding not covered byinstallation.md(CREATE CATALOG requirement, SSO identity reuse, blocked Lakebase ingress, the Phase 6 bug, and bundle placeholder-default overwrite).Test plan
UndefinedTablein app logs.SQLSTATE: 42501instead of 0-row dashboards.tool_registry/request_logs;/api/v1/tools/overviewreturns 200; smoke check fails whentool_registryis dropped.Split out from #18. Original author: @thijs-hakkenberg. Companion PR adds the generic CI/CD pipeline.
This pull request and its description were written by Isaac.