Commit a83c94b
fix: raise DbtRuntimeError when job run terminates with non-success result state (#1477)
<!-- Please review our pull request review process in CONTRIBUTING.md
before your proceed. -->
Resolves #
<!---
Include the number of the issue addressed by this PR above if
applicable.
Example:
resolves #1234
Please review our pull request review process in CONTRIBUTING.md before
your proceed.
-->
### Description
While reading through `JobRunsApi` I noticed that
`_handle_terminal_state`
has a gap in its error handling. When `poll_for_completion` sees a
terminal
lifecycle state and calls into this method, two cases are covered:
`CANCELED`
result state gets a `DbtRuntimeError`, and any non-`TERMINATED`
lifecycle state
(e.g. `SKIPPED`, `INTERNAL_ERROR`) goes through
`_handle_non_terminated_failure`.
What is missing is the case where `life_cycle_state` is `TERMINATED` but
`result_state` is something like `FAILED`, `TIMEDOUT`,
`UPSTREAM_FAILED`, or
`UPSTREAM_CANCELED` — the method just falls off the end and returns
`None`,
so dbt treats the Python model run as successful even though the
notebook
actually failed on Databricks.
This is the most common production failure path: a notebook raises an
unhandled exception, Databricks marks the run `TERMINATED / FAILED`, and
the
dbt run finishes green with no indication that anything went wrong.
The fix adds an `elif` after the existing `life_cycle_state !=
"TERMINATED"`
block. If we are in `TERMINATED` and `result_state` is anything other
than
`None`, `SUCCESS`, or `SUCCESS_WITH_FAILURES`, we raise
`DbtRuntimeError`
with the result state and Databricks state message so the failure is
visible.
Added `test_poll_for_completion__failed` to the existing
`tests/unit/api_client/test_job_runs_api.py` test class, following the
same
pattern as `test_poll_for_completion__cancelled` directly above it. The
test
sets `life_cycle_state = TERMINATED` and `result_state = FAILED`, and
asserts
that a `DbtRuntimeError` containing `result_state FAILED` is raised. On
the
original code the call returns silently and the test fails; after the
fix it
passes.
### Checklist
- [ ] I have run this code in development and it appears to resolve the
stated issue
- [ ] This PR includes tests, or tests are not required/relevant for
this PR
- [ ] I have updated the `CHANGELOG.md` and added information about my
change to the "dbt-databricks next" section.
---------
Co-authored-by: Shubham Dhal <shubham.dhal@databricks.com>1 parent 73c1320 commit a83c94b
3 files changed
Lines changed: 25 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
7 | 10 | | |
8 | 11 | | |
9 | 12 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
617 | 617 | | |
618 | 618 | | |
619 | 619 | | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
620 | 625 | | |
621 | 626 | | |
622 | 627 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
203 | 203 | | |
204 | 204 | | |
205 | 205 | | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
0 commit comments