Skip to content

Commit df1d70e

Browse files
committed
fix(dagster): emit no check result for an audit that did not run
An end-of-build audit skipped because it reads a resource the build did not build has passed: false in the JSON checks, so Dagster reported it as a failed asset check, even in a green build. The Dagster integration now emits no check result for a skipped audit, which Dagster shows as not evaluated, and logs the audit and its skip reason instead. The JSON shape is unchanged.
1 parent fd4ba57 commit df1d70e

4 files changed

Lines changed: 102 additions & 0 deletions

File tree

‎crates/sqlbuild-python/src/bindings/_helpers/dagster_invocation/results.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ const MATERIALIZE: &str = "materialize";
2828
const MATERIALIZATION: &str = "materialization";
2929
const CHECK: &str = "check";
3030
const AUDIT_CHECK_KIND: &str = "audit";
31+
/// The status of an audit that did not run; Dagster gets no check result for it.
32+
const SKIPPED_CHECK_STATUS: &str = "skipped";
3133

3234
/// DAG nodes upstream first, or in artifact order when edges form a cycle.
3335
pub(crate) fn sorted_nodes<'py>(dag: &Bound<'py, PyAny>) -> PyResult<Vec<Bound<'py, PyAny>>> {
@@ -283,6 +285,9 @@ fn check_plans<'py>(
283285
) -> PyResult<(Vec<Bound<'py, PyAny>>, HashSet<CheckKey>)> {
284286
let mut seen: HashSet<CheckKey> = seen;
285287
let py: Python<'py> = dag.py();
288+
if get(check, "status")?.eq(SKIPPED_CHECK_STATUS)? {
289+
return Ok((Vec::new(), seen));
290+
}
286291
let dag_check: Option<Bound<'py, PyAny>> = dag_check_for(dag, check)?;
287292
let (asset_ids, check_name): (Vec<String>, String) = match &dag_check {
288293
Some(dag_check) if !check_enabled(dag_check)? => return Ok((Vec::new(), seen)),

‎src/sqlbuild/integrations/dagster/_helpers/invocation.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
_CHECK_RESULT: str = "check"
2525
_ASSET_MATERIALIZATION: str = "materialization"
26+
_SKIPPED_CHECK_STATUS: str = "skipped"
2627

2728

2829
def _log_invocation(*, context: Any, invocation: SqlBuildCliInvocation) -> None:
@@ -42,6 +43,21 @@ def _log_invocation(*, context: Any, invocation: SqlBuildCliInvocation) -> None:
4243
logger.info(" %s", line)
4344

4445

46+
def _log_skipped_checks(*, context: Any, payload: Mapping[str, Any]) -> None:
47+
"""Log each audit that did not run and why; Dagster gets no check result for it."""
48+
49+
logger: Any = getattr(context, "log", None)
50+
if logger is None:
51+
return
52+
for check in payload.get("checks", ()):
53+
if check.get("status") == _SKIPPED_CHECK_STATUS:
54+
logger.info(
55+
"SQLBuild audit %s did not run: %s",
56+
check.get("name"),
57+
check.get("skip_reason") or "skipped",
58+
)
59+
60+
4561
def _start_stream_future(
4662
*,
4763
executor: ThreadPoolExecutor,
@@ -178,6 +194,7 @@ def _build_results_from_execution_payload(
178194
command: tuple[str, ...],
179195
context: Any,
180196
) -> tuple[Any, ...]:
197+
_log_skipped_checks(context=context, payload=payload)
181198
return _dagster_events(
182199
dg=dg,
183200
plans=dagster_payload_results(

‎tests/e2e/src/sqlbuild/integrations/dagster/_test_types.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,3 +152,12 @@ class DagsterAuditAttachmentChecksE2ETestCase:
152152
description: str
153153
command: tuple[str, ...]
154154
expected_checked_assets: tuple[tuple[str, ...], ...]
155+
156+
157+
@dataclass(frozen=True)
158+
class DagsterSkippedAuditCheckE2ETestCase:
159+
"""A green build whose end audit is skipped, and the check Dagster must not report as failed."""
160+
161+
description: str
162+
skipped_check_name: str
163+
expected_log_line: str

‎tests/e2e/src/sqlbuild/integrations/dagster/test_audit_attachment_checks.py‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,38 @@
1919
)
2020
from tests.e2e.src.sqlbuild.integrations.dagster._test_types import (
2121
DagsterAuditAttachmentChecksE2ETestCase,
22+
DagsterSkippedAuditCheckE2ETestCase,
2223
)
2324
from tests.e2e.src.sqlbuild.integrations.dagster.helpers import (
2425
ATTACHMENT_CHECK_PROJECT_FILES,
2526
AuditCheckOutcome,
2627
audit_check_outcome,
2728
)
2829

30+
_SKIPPED_AUDIT_PROJECT_FILES: dict[str, str] = {
31+
"sqlbuild_project.toml": (
32+
'name = "skipped_audit_checks"\nadapter = "duckdb"\n\n'
33+
'[connection]\ndatabase = "warehouse.duckdb"\n\n[defaults]\nmaterialized = "table"\n'
34+
),
35+
"hooks/python/skips.py": (
36+
"from sqlbuild.hooks import hook\n\n\n"
37+
"@hook\ndef skip_model(ctx):\n"
38+
" '''Test hook skip_model.'''\n"
39+
" return ctx.skip(reason='paused')\n"
40+
),
41+
"models/customers.sql": (
42+
"MODEL (description 'Test model customers.', pre_hooks [python(\"skip_model\")]);\n"
43+
"SELECT 1 AS customer_id\n"
44+
),
45+
"models/orders.sql": "MODEL (description 'Test model orders.');\nSELECT 1 AS order_id\n",
46+
"audits/singular/orders_have_customers.sql": (
47+
"AUDIT ();\n"
48+
'SELECT o.order_id FROM __ref("orders") AS o\n'
49+
'LEFT JOIN __ref("customers") AS c ON c.customer_id = o.order_id\n'
50+
"WHERE c.customer_id IS NULL\n"
51+
),
52+
}
53+
2954

3055
@pytest.mark.parametrize(
3156
"test_case",
@@ -137,3 +162,49 @@ def sqlbuild_attachment_checks(context: AssetExecutionContext) -> Iterator[objec
137162
assert checks.declared <= checks.evaluated
138163
assert checks.all_passed is True
139164
assert "integration output degraded" not in capsys.readouterr().err
165+
166+
167+
@pytest.mark.parametrize(
168+
"test_case",
169+
[
170+
DagsterSkippedAuditCheckE2ETestCase(
171+
description="an end audit over a hook-skipped first-run model is not a failed check",
172+
skipped_check_name="audit__orders_have_customers",
173+
expected_log_line=(
174+
"SQLBuild audit orders_have_customers did not run: reads customers, which was not "
175+
"built"
176+
),
177+
),
178+
],
179+
ids=lambda case: case.description,
180+
)
181+
def test_given_skipped_end_audit_when_building_through_dagster_then_no_failed_check_is_reported(
182+
test_case: DagsterSkippedAuditCheckE2ETestCase,
183+
tmp_path: Path,
184+
capsys: pytest.CaptureFixture[str],
185+
) -> None:
186+
project_dir: Path = prepare_inline_project(
187+
tmp_path=tmp_path,
188+
project_name="skipped_audit_checks",
189+
repo_files=_SKIPPED_AUDIT_PROJECT_FILES,
190+
)
191+
sqlbuild_project: SqlBuildProject = SqlBuildProject(
192+
project_dir=project_dir, sqb_command=(str(REPO_ROOT / ".venv" / "bin" / "sqb"),)
193+
)
194+
sqlbuild_project.prepare()
195+
196+
@sqlbuild_assets(project=sqlbuild_project, required_resource_keys={"sqb"})
197+
def sqlbuild_skipped_audit(context: AssetExecutionContext) -> Iterator[object]:
198+
yield from context.resources.sqb.cli(["build"], context=context).stream()
199+
200+
result: ExecuteInProcessResult = materialize(
201+
[sqlbuild_skipped_audit],
202+
resources={"sqb": SqlBuildCliResource(project_dir=sqlbuild_project)},
203+
)
204+
205+
checks: AuditCheckOutcome = audit_check_outcome(result=result, assets=sqlbuild_skipped_audit)
206+
assert checks.success is True
207+
assert test_case.skipped_check_name in {name for _, name in checks.declared}
208+
assert test_case.skipped_check_name not in {name for _, name in checks.evaluated}
209+
assert checks.all_passed is True
210+
assert test_case.expected_log_line in capsys.readouterr().err

0 commit comments

Comments
 (0)