Skip to content

fix(workflows): gateway DDL + billing perm-error + tools tables + Lakebase smoke check - #20

Merged
bkaankuguoglu merged 7 commits into
mainfrom
fix/lakebase-ddl-billing-tools-smoke
Jun 9, 2026
Merged

fix(workflows): gateway DDL + billing perm-error + tools tables + Lakebase smoke check#20
bkaankuguoglu merged 7 commits into
mainfrom
fix/lakebase-ddl-billing-tools-smoke

Conversation

@bkaankuguoglu

Copy link
Copy Markdown
Member

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_models task, workload_class columns) is preserved — the original #18 branch predated it.

1. Gateway DDL transaction fix (02_sync_to_lakebase.py Phase 6)

On a fresh DB, Phase 6 issued an ALTER TABLE gateway_usage_hourly before the matching CREATE TABLE. psycopg2 marks the whole transaction aborted on the first error, so the trailing CREATE TABLEs silently no-op'd — leaving the DB with neither gateway_usage_daily nor gateway_usage_hourly, while the discovery job still reported SUCCESS. The deployed app then logged an unbounded stream of UndefinedTable errors and rendered the Gateway page empty.

Fix: reorder so all CREATE TABLEs precede CREATE INDEX/ALTER, and run one transaction per DDL (per-statement commit(), rollback()-and-continue on failure) so a first-run ALTER can't poison sibling DDLs. Verified: both tables land with rows (21,303 / 2,606), no more UndefinedTable.

2. Surface SQL permission errors in 09_discover_billing.py

_execute_sql returned [] on any non-SUCCEEDED state — including INSUFFICIENT_PERMISSIONS. A missing SELECT ON system.billing.list_prices grant silently produced 0-row cost dashboards (Cost Overview / Endpoint Costs / All Products) for weeks while token usage worked, and discovery still reported SUCCESS.

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-line GRANT, documented in the RCA — environmental, not code.)

3. App-managed table DDL moved into the workflow (tools_service.py + 02_sync Phase 7)

tool_registry / request_logs were 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/overview returned 500, the Tools page rendered empty, and /api/v1/tools/sync returned HTTP 200 regardless.

Fix: new Phase 7 in 02_sync_to_lakebase.py creates 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 (calls ensure_tools_tables() first) and switches the catch-all from logger.warning to logger.exception so tracebacks land in app logs.

4. Lakebase smoke check (workflows/10_smoke_check_lakebase.py)

A new smoke_check_lakebase task asserts every Lakebase table the dashboard reads from exists with the expected content — the regression gate the bugs above would have caught. Tables are bucketed REQUIRED (exist + ≥1 row), EXPECTED (exist, 0 rows → WARN), OPTIONAL (app-managed, not asserted). It raises directly (not via dbutils.notebook.exit(), which would mask the failure) so result_state flips to FAILED for CI to gate on, and inlines the per-table breakdown into the exception message (since jobs/get-run-output only round-trips error/error_trace). Wired in databricks.yml, depending on sync_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)

  • In-workspace SP-grant fallback (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.sh points to it.
  • RCA log (docs/rca/2026-06-06-deploy-non-trivial-fixes.md): deployment-blocking issues from the onboarding not covered by installation.md (CREATE CATALOG requirement, SSO identity reuse, blocked Lakebase ingress, the Phase 6 bug, and bundle placeholder-default overwrite).

Test plan

  • Phase 6 fix: both gateway tables exist with non-zero rows after a full discovery run; no UndefinedTable in app logs.
  • Billing fix: with the grant, all five billing queries return rows and sync 1:1; with the grant revoked, discovery fails loud with SQLSTATE: 42501 instead of 0-row dashboards.
  • Tools fix: Phase 7 creates tool_registry/request_logs; /api/v1/tools/overview returns 200; smoke check fails when tool_registry is dropped.
  • Smoke check passes on healthy Lakebase, fails loud (with per-table breakdown in the error) on a broken one.
  • (Reviewers) Run on a clean target where neither gateway table existed.

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.

thijshakkenbergecolab and others added 5 commits June 9, 2026 18:58
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>
…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
@bkaankuguoglu
bkaankuguoglu merged commit cf485b8 into main Jun 9, 2026
@bkaankuguoglu
bkaankuguoglu deleted the fix/lakebase-ddl-billing-tools-smoke branch June 9, 2026 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants