From 8ca5837dcc899d89fb0304b6a7dae30f46195cf4 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sat, 5 Sep 2026 10:42:42 +0200 Subject: [PATCH] fix: report a warehouse-lookup failure as the failure it is, and correct the boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dbt_warehouse_exists` fails three ways — no such warehouse, the query itself, and a setting with no `resource_path` — and all three became a 400 blaming the user's warehouse name. A pool timeout mid-deploy told a retrying sync that a transient server error was a permanent client one. Only `NotFound` is the annotation's fault now. The known-boundary paragraph claimed a flow-runner run still cascades. It does not: it is routed by `flow_step_id`, which `is_eligible_kind` rejects, as `asset_trigger_dispatch.rs` pins. Recording and cascading are decided separately, so the paragraph now names all three routes rather than merging two of them — and the row it omitted, an ordinary flow step, which records and never cascades. E2E item 7 said "deployable" where the rule is "wakeable": with only the dbt project reading the relation the producer set is empty, which deploys fine. Co-Authored-By: Claude Opus 5 (1M context) --- backend/windmill-api-scripts/src/scripts.rs | 11 ++++++--- docs/dbt-runtime.md | 25 ++++++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index 1fa7926495..5b95d43407 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -1116,13 +1116,18 @@ async fn validate_dbt_relation( ))); } let warehouse = relation.split('/').next().unwrap_or_default(); + // Only the resolver's own "no such warehouse" is the annotation's fault. Its + // other failures are the query and a malformed setting, and blanket-mapping + // those to 400 would tell a retrying sync that a pool timeout is a permanent + // client error. windmill_common::workspaces::dbt_warehouse_exists(db, w_id, warehouse) .await - .map_err(|e| { - Error::BadRequest(format!( + .map_err(|e| match e { + Error::NotFound(_) => Error::BadRequest(format!( "{what} `dbt://{relation}` names a warehouse this workspace does not \ configure: {e}" - )) + )), + other => other, }) } diff --git a/docs/dbt-runtime.md b/docs/dbt-runtime.md index b7ec709e04..f68e48993f 100644 --- a/docs/dbt-runtime.md +++ b/docs/dbt-runtime.md @@ -707,12 +707,20 @@ warehouse answers to is not a namespace, it strands the write on a node nothing else reaches. Known boundary, shared with every other runtime pipeline annotation: the record -is written from the normal execution path, which a job handed to a **dedicated -worker or a flow runner** never enters — those bypass it exactly as they bypass -`// partitioned` resolution. Such a run performs its write and cascades (the -fan-out reads the deploy-time `asset` rows) but records no row, so the relation -shows no last writer. Fixing it is one change for all of those annotations, not -this one. +is written from the normal execution path, and recording and cascading are decided +separately, so the three routes off it differ. + +* A **dedicated worker** never enters that path — it bypasses the record exactly + as it bypasses `// partitioned` resolution — while its job is still a top-level + `Script`, so the fan-out (which reads the deploy-time `asset` rows) runs. It + cascades and records nothing, leaving the relation with no last writer. +* A **flow runner** bypasses the path too, and is routed by `flow_step_id`, which + `is_eligible_kind` rejects. Neither record nor cascade. +* An ordinary **flow step** does enter the path and its kind is `Script`, so it + records — and is still a flow step, so it never cascades. + +Fixing the recording half is one change for every runtime pipeline annotation, +not this one. A `# on dbt://` subscription is held to the same relation a producer is — a whole `//` under a configured warehouse, checked @@ -1292,8 +1300,9 @@ Against a real dbt project (jaffle_shop shape) and the local Postgres: 7. **Declared write**: a native `// materialize manual dbt://` script and a dbt project reading that relation as a `source` render as one node; a run of the script records its materialization and wakes a - `# on dbt://` subscriber — a subscription only that producer's - existence makes deployable (see "no cascade *from* dbt"). + `# on dbt://` subscriber — a subscription only that producer makes + wakeable, the dbt project reading the relation being no producer of it (see + "no cascade *from* dbt"). 8. **Selection**: descriptor `select`/`exclude`, and a run-arg override, each build only the expected subset. 9. **Dynamic descriptors**: a `{{ }}` placeholder in `vars` re-ingests the graph