Skip to content

Fixes 29824: ingest dbt results with a null message and stop compile-only stubs shadowing executed results - #31138

Open
TeddyCr wants to merge 2 commits into
open-metadata:mainfrom
TeddyCr:ISSUE-29824
Open

Fixes 29824: ingest dbt results with a null message and stop compile-only stubs shadowing executed results#31138
TeddyCr wants to merge 2 commits into
open-metadata:mainfrom
TeddyCr:ISSUE-29824

Conversation

@TeddyCr

@TeddyCr TeddyCr commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Describe your changes:

Fixes #29824

I worked on the dbt test-result ingestion path because, after upgrading to 1.13.0, dbt test cases were still created from manifest.json but their results from run_results.json silently never appeared.

There are two independent defects behind the report, both 1.13.0-only regressions, and both are fixed here.

Defect 1 — the null-message guard was too broad. Commit 964ac7578271 (#26812) added a guard in add_dbt_test_result() that skips any result with a falsy message, to filter out phantom dbt run entries. But dbt only populates message on failure/warn, so a genuine passing data test emits status="pass", message=null and was dropped along with the stubs. DbtTestSuccessEnum defines both SUCCESS = "success" (compile-only) and PASS = "pass" (really executed); the guard checked neither.

Defect 2 — compile-only stubs could shadow real results. Reported by @ziggekatten in the issue thread and not covered by #29828. _get_latest_result() picked among duplicate unique_id entries purely by execute.completed_at. A dbt docs generate artifact written after a dbt test one wins on timestamp with its status="success" stub, so the real result was discarded before add_dbt_test_result() ever ran. This only affects blob-storage config sources — DbtLocalConfig and DbtHttpConfig structurally cannot produce two run-results files.

Both call sites now share one predicate, is_compiled_only_result() (status == "success" AND not message), so the selector can never hand the ingester a result it will then drop — that divergence is the bug class.

Relationship to #29828: this supersedes it. #29828 fixes Defect 1 only. Reverting just this PR's Defect 2 change reproduces #29828's exact shape, and the end-to-end test still fails assert 0 == 1 — so #29828 alone leaves S3/GCS/Azure deployments broken. Its RCA for Defect 1 is correct and was reused, with credit in the Defect 1 commit.

Why status and not failures: failures is not in REQUIRED_RESULTS_KEYS, so remove_run_result_non_required_keys() strips it before parsing. status is the only discriminator that survives. It is safe to dereference: all 9 run-results models in collate_dbt_artifacts_parser declare status as required=True, non-nullable, enum-typed.

Type of change:

  • Bug fix

High-level design:

N/A — small change (3 files, +292/−5).

Tests:

Use cases covered

  • A dbt test that passes with status="pass" and message=null is ingested as a Success result (previously dropped).
  • Same for status="fail" / status="warn" with a null message.
  • When several run_results.json files are read from one blob prefix, an executed result (pass/fail/warn) is preferred over a later compile-only status="success" stub for the same unique_id.
  • Preference holds regardless of ordering, and when no candidate has a usable execute timestamp.
  • Compile-only stubs are still skipped when nothing was executed — the Fixes #26785 - skip compiled-only dbt test results with null message #26812 behaviour is preserved.

Unit tests

  • I added unit tests for the new/changed logic.
  • Files updated: ingestion/tests/unit/test_dbt.py (+268, 12 new tests across TestAddDbtTestResultNullMessage and TestGetLatestResultPrefersExecutedResults)
  • python -m pytest tests/unit/test_dbt.py183 passed
  • Each test was verified RED per defect before the fix. Reverting the Defect 1 guard → 4 failed; reverting the Defect 2 filter → 4 failed.
  • The tests assert on the TestCaseResult handed to the OpenMetadata client — status, result, testResultValue, timestamp and a real fqn-built test_case_fqn — after running a real run_results.json through the production key-stripper and the real collate_dbt_artifacts_parser. The fqn module is deliberately not patched, so the FQN assertion is real rather than tautological.

Backend integration tests

  • Not applicable (no backend API changes). The defect is entirely client-side: the requests were dropped before reaching the API.

Ingestion integration tests

  • Not applicable — no live-artifact integration harness exists for the dbt connector; covered by unit tests over really-parsed artifacts.

Playwright (UI) tests

  • Not applicable (no UI changes).

Manual testing performed

No live stack was used; these steps are for a reviewer who wants one. The two defects need different config sources — Defect 2 requires more than one run_results.json in a single DbtFiles, which only the blob-storage sources can produce. Using Local for Defect 2 will silently not reproduce it.

Defect 1 (single artifact — Local is fine)

  1. Run dbt test on a project (e.g. jaffle_shop). Confirm target/run_results.json has a test node with "status": "pass", "message": null, "failures": 0.
  2. Configure a database service ingestion with dbtConfigSource type Local, setting dbtManifestFilePath and dbtRunResultsFilePath.
  3. Run metadata ingest -c <workflow>.yaml.
  4. Expected (fixed): the table's Test Cases tab shows a Success result row per dbt test.
  5. Expected (pre-fix): test cases exist with no result rows, and the log shows Skipping compiled-only test result for '<node>' (message is null).

Defect 2 (multiple artifacts — S3/GCS/Azure, or MinIO locally)

  1. Produce two artifact sets, keeping both run-results files:
    dbt test            # status="pass", message=null, failures=0
    cp target/run_results.json ./stage/run_results_test.json
    cp target/manifest.json    ./stage/manifest.json
    sleep 5
    dbt docs generate   # same unique_id, status="success", message=null
    cp target/run_results.json ./stage/run_results_docs.json
    Both filenames must contain run_results — that substring is what makes the reader pick up both.
  2. Upload all three under one prefix, e.g. s3://<bucket>/dbt/.
  3. Configure ingestion with dbtConfigSource type S3 (or GCS/Azure) at that prefix. Confirm from the debug log that both run-results files are read.
  4. Run metadata ingest -c <workflow>.yaml.
  5. Expected (fixed): the test case shows a Success result whose timestamp matches the dbt test run, not the later dbt docs generate run.
  6. Expected (pre-fix): no result row, and the log shows the compiled-only skip — the docs stub won selection and was then discarded.

Negative check (both)
7. Run a job that only does dbt run and confirm no bogus result rows appear — compiled-only stubs must still be skipped.

UI screen recording / screenshots:

Not applicable.

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: N/A — no schema change, so no migration needed.
  • For UI changes: N/A — no UI changes.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.
  • I have added a test that covers the exact scenario we are fixing.

Known limitations

  • Non-executed node statuses other than success (skipped, partial success, and dbt Cloud's no-op) with a null message are now ingested as Aborted, where 1.13.0 dropped them. This is deliberate: it restores pre-Fixes #26785 - skip compiled-only dbt test results with null message #26812 (1.12.x) behaviour. Narrowing to an allow-list of executed statuses would re-introduce exactly the over-broad filtering that caused this issue.
  • is_compiled_only_result carries one reportMissingParameterType basedpyright warning, matching 11 neighbouring helpers in dbt_utils.py. There is no honest shared type — the parser exposes 9 unrelated result classes with no common base, and it is imported lazily inside get_dbt_objects(). basedpyright reports zero new errors versus the unmodified base.
  • The pre-existing TestGetLatestResult class builds MagicMocks whose .message is auto-truthy, so the new filter is a no-op there — those tests stay green for reasons unrelated to this change and are not selector coverage. The new TestGetLatestResultPrefersExecutedResults class uses really-parsed artifacts.

🤖 Generated with Claude Code

TeddyCr and others added 2 commits August 6, 2026 13:04
…essage

PR open-metadata#26812 added a guard in add_dbt_test_result() that skipped any
run_results entry whose `message` was null, to drop the compiled-only
test nodes that `dbt run` writes with status="success". The guard never
looked at `status`, so it also dropped genuine executed data tests: dbt
only populates `message` on failure/warn, so a passing test arrives as
status="pass", message=null. That is why dbt test results stopped
appearing after the 1.13.0 upgrade.

Move the discrimination into `is_compiled_only_result()` in dbt_utils and
key it on both `status` and `message`, so only status="success" entries
with no message are treated as compiled-only. `status` is safe to
dereference: it is a required, non-nullable Enum on every run-results
model the shared parser can produce (v1-v6 plus the two cloud variants),
and all connector variants parse through DbtServiceSource.get_dbt_objects.
`failures` is not usable as the discriminator because it is stripped by
REQUIRED_RESULTS_KEYS before parsing.

Supersedes the same narrowing proposed in PR open-metadata#29828 by ayush-shah.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…xecuted results

_get_latest_result() de-duplicated a unique_id across multiple
run_results.json files purely by `execute.completed_at`. A project that
keeps both a `dbt test` artifact and a later `dbt docs generate` artifact
therefore selected the docs run's compile-only stub (status="success",
message=null), and add_dbt_test_result() then dropped it as compiled-only
- so the real pass/fail result never reached OpenMetadata even with the
message guard fixed. Reported on 1.13.1 / dbt 1.11 by ziggekatten.

Filter compile-only entries out of the candidate set before ranking by
timestamp, reusing the same is_compiled_only_result() predicate as the
ingestion guard so the two can never disagree. Sharing the predicate is
load-bearing rather than incidental: if the selector's notion of
"compile-only" diverged from the ingester's, the selector could hand over
a result the ingester then drops, which is exactly this bug.

When every match is a stub the previous behaviour is kept, since there is
nothing better to pick. The no-parseable-timestamp fallback now returns
the first *executed* candidate rather than the first candidate overall;
that behaviour change is pinned by
test_executed_result_wins_when_no_timestamp_is_usable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@TeddyCr
TeddyCr requested a review from a team as a code owner August 6, 2026 21:18
Copilot AI review requested due to automatic review settings August 6, 2026 21:18
@TeddyCr TeddyCr added the safe to test Add this label to run secure Github workflows on PRs label Aug 6, 2026
Comment thread ingestion/src/metadata/ingestion/source/database/dbt/dbt_utils.py
@gitar-bot

gitar-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Fixes dbt test ingestion by refining the null-message guard and ensuring executed results are preferred over compile-only stubs. Consider adding a safety check for status.value to prevent potential attribute errors.

✅ 1 resolved
Bug: _get_latest_result now dereferences status.value unguarded

📄 ingestion/src/metadata/ingestion/source/database/dbt/dbt_utils.py:854 📄 ingestion/src/metadata/ingestion/source/database/dbt/metadata.py:645
is_compiled_only_result does dbt_test_result.status.value, and it is now invoked from _get_latest_result (called by add_dbt_tests), which — unlike add_dbt_test_result — has no surrounding try/except. If any parsed run-result ever has status=None (the PR relies on all 9 parser models declaring status as required/non-nullable, which was not verifiable in this checkout), this would raise an unhandled AttributeError and abort selection for that node, a path the old timestamp-only selector did not touch. If the non-null guarantee is certain this is a non-issue; otherwise consider getattr(result.status, "value", None).

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds regression coverage and logic adjustments to correctly distinguish executed dbt test results from compiled-only stubs, ensuring OpenMetadata ingests the right test outcome even when multiple dbt artifacts exist.

Changes:

  • Add helper utilities + new regression tests for issue #29824 scenarios (null message, compiled-only stubs, latest-result selection).
  • Update _get_latest_result to prefer executed results over compiled-only entries.
  • Centralize compiled-only detection in a new is_compiled_only_result utility and use it in ingestion logic.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File Description
ingestion/tests/unit/test_dbt.py Adds regression tests + payload helpers to simulate real dbt artifacts and validate ingestion/selection behavior.
ingestion/src/metadata/ingestion/source/database/dbt/metadata.py Updates latest-result selection and ingestion skip logic to use compiled-only detection.
ingestion/src/metadata/ingestion/source/database/dbt/dbt_utils.py Introduces is_compiled_only_result helper used by selection + ingestion.

self.assertIs(got, new_result)


class TestGetLatestResultPrefersExecutedResults:
source.metadata.add_test_case_results.assert_called_once()


class TestAddDbtTestResultNullMessage:
return primary_table_fqn


def is_compiled_only_result(dbt_test_result) -> bool:
alone cannot be used as the discriminator (issue #29824). ``failures`` would be
the other signal but it is dropped by ``REQUIRED_RESULTS_KEYS`` before parsing.
"""
return not dbt_test_result.message and dbt_test_result.status.value == DbtTestSuccessEnum.SUCCESS.value
Comment on lines +3292 to +3294
assert kwargs["test_results"].timestamp.root == datetime_to_timestamp(
datetime(2026, 7, 24, 7, 0, 0), milliseconds=True
)
Comment on lines +3980 to +3982
assert test_case_result.timestamp.root == datetime_to_timestamp(
datetime(2026, 7, 24, 9, 0, 0), milliseconds=True
)
Comment thread ingestion/tests/unit/test_dbt.py
Comment thread ingestion/tests/unit/test_dbt.py
@gitar-bot

gitar-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

Verified — the finding was inaccurate on both points.

  1. add_dbt_tests (and therefore _get_latest_result) is called at metadata.py:818, inside the try: opened at :809, and any exception there is caught by except Exception as exc: at :935, producing a per-node StackTraceError just like every other node-parsing failure. There's no asymmetry with add_dbt_test_result's error handling.

  2. dbt_test_result.status.value isn't a new dereference introduced by this PR — it already exists on the base commit (metadata.py:1995, :1998), and _get_latest_result predates this change. parse_run_results performs full Pydantic validation against the dbt-artifacts-parser models, where status is a required, non-Optional, enum-typed field, so a status=None input fails at parse time with a ValidationError rather than reaching is_compiled_only_result.

No code change needed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dbt test case results not appearing in OpenMetadata after 1.13.0 upgrade

2 participants