Skip to content

Commit

Permalink
Ensure BaseRunner handles nodes without build_path
Browse files Browse the repository at this point in the history
Some nodes, like SourceDefinition nodes, don't have a `build_path` property.
This is problematic because we take in nodes with no type checking, and
assume they have properties sometimes, like `build_path`. This was just
the case in BaseRunner's `_handle_generic_exception` and
`_handle_interal_exception` methods. Thus to stop dbt from crashing when
trying to handle an exception related to a node without a `build_path`,
we added an private method to the BaseRunner class for safely trying
to get `build_path`.
  • Loading branch information
QMalcolm committed Mar 26, 2024
1 parent 952cca8 commit 7b0ff31
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 26 deletions.
7 changes: 5 additions & 2 deletions core/dbt/task/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ def __init__(self, config, adapter, node, node_index, num_nodes) -> None:
def compile(self, manifest: Manifest) -> Any:
pass

def _node_build_path(self) -> Optional[str]:
return self.node.build_path if hasattr(self.node, "build_path") else None

def get_result_status(self, result) -> Dict[str, str]:
if result.status == NodeStatus.Error:
return {"node_status": "error", "node_error": str(result.message)}
Expand Down Expand Up @@ -339,15 +342,15 @@ def _handle_catchable_exception(self, e, ctx):
def _handle_internal_exception(self, e, ctx):
fire_event(
InternalErrorOnRun(
build_path=self.node.build_path, exc=str(e), node_info=get_node_info()
build_path=self._node_build_path(), exc=str(e), node_info=get_node_info()
)
)
return str(e)

def _handle_generic_exception(self, e, ctx):
fire_event(
GenericExceptionOnRun(
build_path=self.node.build_path,
build_path=self._node_build_path(),
unique_id=self.node.unique_id,
exc=str(e),
node_info=get_node_info(),
Expand Down
29 changes: 29 additions & 0 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import pytest

from dbt.artifacts.resources import Quoting, SourceConfig
from dbt.artifacts.resources.types import NodeType
from dbt.contracts.graph.nodes import SourceDefinition


@pytest.fixture
def basic_parsed_source_definition_object():
return SourceDefinition(
columns={},
database="some_db",
description="",
fqn=["test", "source", "my_source", "my_source_table"],
identifier="my_source_table",
loader="stitch",
name="my_source_table",
original_file_path="/root/models/sources.yml",
package_name="test",
path="/root/models/sources.yml",
quoting=Quoting(),
resource_type=NodeType.Source,
schema="some_schema",
source_description="my source description",
source_name="my_source",
unique_id="test.source.my_source.my_source_table",
tags=[],
config=SourceConfig(),
)
24 changes: 24 additions & 0 deletions tests/unit/task/test_base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from dbt.task.base import BaseRunner
from dbt.contracts.graph.nodes import SourceDefinition


class MockRunner(BaseRunner):
def compile(self):
pass


class TestBaseRunner:
def test_handle_generic_exception_handles_nodes_without_build_path(
self, basic_parsed_source_definition_object: SourceDefinition
):
# Source definition nodes don't have `build_path` attributes. Thus, this
# test will fail if _handle_generic_exception doesn't account for this
runner = MockRunner(
config=None,
adapter=None,
node=basic_parsed_source_definition_object,
node_index=None,
num_nodes=None,
)
assert not hasattr(basic_parsed_source_definition_object, "build_path")
runner._handle_generic_exception(Exception("bad thing happened"), ctx=None)
24 changes: 0 additions & 24 deletions tests/unit/test_contracts_graph_parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -1954,30 +1954,6 @@ def basic_parsed_source_definition_dict():
}


@pytest.fixture
def basic_parsed_source_definition_object():
return SourceDefinition(
columns={},
database="some_db",
description="",
fqn=["test", "source", "my_source", "my_source_table"],
identifier="my_source_table",
loader="stitch",
name="my_source_table",
original_file_path="/root/models/sources.yml",
package_name="test",
path="/root/models/sources.yml",
quoting=Quoting(),
resource_type=NodeType.Source,
schema="some_schema",
source_description="my source description",
source_name="my_source",
unique_id="test.source.my_source.my_source_table",
tags=[],
config=SourceConfig(),
)


@pytest.fixture
def complex_parsed_source_definition_dict():
return {
Expand Down

0 comments on commit 7b0ff31

Please sign in to comment.