Skip to content

Commit

Permalink
Add type hinting to on_skip and fix unhandled potential runtime error
Browse files Browse the repository at this point in the history
In adding type hinting to `on_skip`, and `do_skip`, a potential runtime
error was uncovered wherein we were accessing `self.skip_cause.status` in
`on_skip` when the `skip_cause` can be `None`. On that edge case, a
runtime error would be raised given how we were attempting to access
the `status` property. We now handle this edge case, and we only discovered
it because of the additional typing.
  • Loading branch information
QMalcolm committed Aug 26, 2024
1 parent 0f81bd1 commit 37b197c
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions core/dbt/task/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ def _skip_caused_by_ephemeral_failure(self) -> bool:
return False
return self.skip_cause.node.is_ephemeral_model

def on_skip(self):
def on_skip(self) -> RunResult:
schema_name = getattr(self.node, "schema", "")
node_name = self.node.name

Expand All @@ -465,7 +465,9 @@ def on_skip(self):
relation=node_name,
index=self.node_index,
total=self.num_nodes,
status=self.skip_cause.status,
status=(
self.skip_cause.status if self.skip_cause is not None else "unknown"
),
)
)
# skip_cause here should be the run_result from the ephemeral model
Expand Down Expand Up @@ -499,7 +501,7 @@ def on_skip(self):
node_result = RunResult.from_node(self.node, RunStatus.Skipped, error_message)
return node_result

def do_skip(self, cause=None):
def do_skip(self, cause: Optional[RunResult] = None) -> None:
self.skip = True
self.skip_cause = cause

Expand Down

0 comments on commit 37b197c

Please sign in to comment.