Skip to content

Fixes #29824: Ingest dbt pass results with null message - #29828

Open
ayush-shah wants to merge 2 commits into
mainfrom
29824-fix-dbt-null-message-test-results
Open

Fixes #29824: Ingest dbt pass results with null message#29828
ayush-shah wants to merge 2 commits into
mainfrom
29824-fix-dbt-null-message-test-results

Conversation

@ayush-shah

@ayush-shah ayush-shah commented Jul 8, 2026

Copy link
Copy Markdown
Member

Describe your changes:

Fixes #29824

RCA

PR #26812 added a guard in add_dbt_test_result() to skip dbt run artifacts where message is null/empty. That fixed compiled-only dbt entries produced by dbt run, where test nodes can appear as status=success with message=null even though no test SQL executed.

The guard was broader than the RCA: dbt run_results.json can also contain executed data tests with status=pass and message=null. dbt's run-results docs show generic data test entries with status: pass, message: null, and failures: 0.

Because OpenMetadata strips non-required run-result keys before parsing, failures is not available in this method. status is the discriminator we still have here.

Fix

  • Keep skipping compiled-only entries with status=success and missing message.
  • Allow executed passing test results with status=pass and missing message to be ingested.
  • Add regression coverage for status=pass, message=None.

Type of change:

  • Bug fix

Checklist:

  • My PR title is Fixes <issue-number>: <short explanation>
  • I have added a test that covers the exact scenario we are fixing.
  • For JSON Schema changes: not applicable.

Verification:

  • Parser-backed regression: a run_results.json entry with status="pass", message=null, execute timing, and a data-test unique_id is parsed with parse_run_results before ingestion.
  • cd ingestion && nox --no-venv -s unit-tests -- tests/unit/test_dbt.py::TestAddDbtTestResultWithMissingMessage — 6 passed (Python 3.10).
  • cd ingestion && nox --no-venv -s unit-tests — 7,681 tests; 0 failures/errors, 13 skipped.
  • make py_format && make py_format_check
  • cd ingestion && nox --no-venv -s static-checks — 0 errors; only existing repository warnings.
  • git diff --check
  • mvn -B clean package -pl openmetadata-service -am -Pstatic-code-analysis -DfailIfNoTests=false -Dsonar.skip=true — 6,635 tests; 0 failures/errors, 1 skipped.

References:

Greptile Summary

This PR fixes ingestion of passing dbt test results with a null message. The main changes are:

  • Keeps skipping compiled-only results with status="success" and no message.
  • Ingests executed results with status="pass" and no message.
  • Adds direct and parser-backed tests for the corrected behavior.

Confidence Score: 5/5

This looks safe to merge.

  • The guard now distinguishes compiled-only success entries from executed pass results.
  • Tests cover both the retained skip behavior and the newly accepted parser output.
  • No blocking issues were found in the changed code.

Important Files Changed

Filename Overview
ingestion/src/metadata/ingestion/source/database/dbt/metadata.py Narrows the missing-message guard so executed pass results are ingested.
ingestion/tests/unit/test_dbt.py Adds direct and parser-backed tests for null-message passing results.

Reviews (7): Last reviewed commit: "Merge branch 'main' into 29824-fix-dbt-n..." | Re-trigger Greptile

Context used (3)

  • Context used - openmetadata-ui-core-components/CLAUDE.md (source)
  • Context used - CLAUDE.md (source)
  • Context used - AGENTS.md (source)

@ayush-shah
ayush-shah requested a review from a team as a code owner July 8, 2026 03:45
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

@github-actions github-actions Bot added Ingestion safe to test Add this label to run secure Github workflows on PRs labels Jul 8, 2026
@ayush-shah ayush-shah added the skip-pr-checks Bypass PR metadata validation check label Jul 8, 2026
@ayush-shah
ayush-shah force-pushed the 29824-fix-dbt-null-message-test-results branch from 13e5936 to 2fc8c8e Compare July 8, 2026 03:47
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

PR #26812 skipped every dbt test result with a null message to avoid compiled-only dbt run entries. dbt run_results.json can also contain executed passing data tests with status=pass and message=null, so those results were dropped after the 1.13 upgrade.

Keep the compiled-only skip for status=success, and add a regression test for status=pass with a null message.
@ayush-shah
ayush-shah force-pushed the 29824-fix-dbt-null-message-test-results branch from 730719f to ed6b2b7 Compare July 8, 2026 12:40
Copilot AI review requested due to automatic review settings July 8, 2026 12:40

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@ayush-shah

Copy link
Copy Markdown
Member Author

Fresh author summary for current head ed6b2b705225: this narrows the dbt null-message skip so compiled-only status="success" entries are still ignored, while executed status="pass" results with message=null are ingested. Added regression coverage for both the direct result object and parsed run_results.json path. Current validation is green in the live PR checks; ready for reviewer/merge once branch protections are satisfied.

Copilot AI review requested due to automatic review settings July 22, 2026 08:03
@gitar-bot

gitar-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Refines dbt test-result ingestion to correctly process passing tests with null messages while maintaining filters for compiled-only entries. No issues found.

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

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@sonarqubecloud

Copy link
Copy Markdown

@ziggekatten

Copy link
Copy Markdown

Additional case: multiple run_results.json files

We are seeing the same problem with OpenMetadata 1.13.1 and dbt 1.11, but our investigation found an additional case that PR #29828 may not cover.

Our artifact location contains multiple run_results.json files containing the same dbt test.

The actual dbt test execution contains:

{
  "unique_id": "test.datalake.not_null_stg_riksbank__currency_rates_currency",
  "status": "pass",
  "message": null,
  "failures": 0
}

A later dbt docs generate execution contains an entry for the same test:

{
  "unique_id": "test.datalake.not_null_stg_riksbank__currency_rates_currency",
  "status": "success",
  "message": null,
  "failures": null
}

The ingestion code groups results by unique_id, and _get_latest_result() selects the result with the latest timing.execute.completed_at.

Because dbt docs generate runs after dbt test, the later compile-only status: success entry is selected instead of the real status: pass result. It is subsequently skipped, leaving the OpenMetadata test case without a status or last-run timestamp.

PR #29828 fixes the case where a selected pass result has a null message, but it does not appear to address this latest-result selection issue.

Could _get_latest_result() exclude compile-only test entries before selecting the latest execution? For test nodes, the following combination appears to identify those entries:

result.status.value == "success" and result.failures is None

The expected behavior would be:

  • A result with status: pass, message: null, and failures: 0 is ingested as a successful test result.
  • A later compile-only result with status: success and failures: null does not replace an actual pass, fail, warn, or error test execution.

@ayush-shah

Copy link
Copy Markdown
Member Author

Thanks @ziggekatten we will review and get back on the PR you have raised 🙏

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

Labels

Ingestion safe to test Add this label to run secure Github workflows on PRs skip-pr-checks Bypass PR metadata validation check

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

4 participants