Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions python_modules/libraries/dagster-dbt/dagster_dbt/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,19 @@
REFABLE_NODE_TYPES: list[str] = []
else:
if DBT_PYTHON_VERSION is not None:
from dbt.adapters.base.impl import (
BaseAdapter as BaseAdapter,
BaseColumn as BaseColumn,
BaseRelation as BaseRelation,
)
try:
from dbt.adapters.base.impl import (
BaseAdapter as BaseAdapter,
BaseColumn as BaseColumn,
BaseRelation as BaseRelation,
)
except ModuleNotFoundError as e:
raise ModuleNotFoundError(
"A dbt adapter package could not be found.\n\n"
"Please install the appropriate dbt adapter for your data platform "
"(for example: dbt-postgres, dbt-bigquery, or dbt-snowflake).\n\n"
f"Original error:\n{e}"
) from e
Comment on lines +43 to +55

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.

P2 Error message may misdirect users when the base adapter package is missing

dbt.adapters.base.impl lives in the dbt-adapters package, which is a core dependency of dbt-core, not a specific adapter package. If this import fails with ModuleNotFoundError, it means dbt-adapters itself is absent (e.g., a broken or partial install), not that a platform-specific adapter like dbt-postgres is missing. Installing dbt-postgres would indirectly fix the issue (it pulls in dbt-adapters), but a user who already has dbt-postgres installed would be confused by being told to install it again. The message would be more accurate if it mentioned dbt-adapters as the primary fix, with platform adapters as the second suggestion.

from dbt.contracts.results import NodeStatus, TestStatus
from dbt.node_types import NodeType as NodeType

Expand Down
25 changes: 23 additions & 2 deletions python_modules/libraries/dagster-dbt/dagster_dbt/core/resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,13 @@ def my_dbt_op(dbt: DbtCliResource):
dagster_dbt_translator = updated_params.dagster_dbt_translator
selection_args = updated_params.selection_args
indirect_selection = updated_params.indirect_selection
target_path = target_path or self._get_unique_target_path(context=context)
raw_target_path = target_path or self._get_unique_target_path(context=context)
if not isinstance(raw_target_path, Path):
raise ValueError(
f"The 'target_path' argument must be a pathlib.Path, not {type(raw_target_path).__name__!r}."
" Pass a Path object, for example: target_path=Path('target')."
)
target_path = raw_target_path
Comment on lines +649 to +655

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.

P2 The intermediate raw_target_path variable is assigned and then immediately reassigned to target_path without any transformation. The isinstance guard can operate on target_path directly.

Suggested change
raw_target_path = target_path or self._get_unique_target_path(context=context)
if not isinstance(raw_target_path, Path):
raise ValueError(
f"The 'target_path' argument must be a pathlib.Path, not {type(raw_target_path).__name__!r}."
" Pass a Path object, for example: target_path=Path('target')."
)
target_path = raw_target_path
target_path = target_path or self._get_unique_target_path(context=context)
if not isinstance(target_path, Path):
raise ValueError(
f"The 'target_path' argument must be a pathlib.Path, not {type(target_path).__name__!r}."
" Pass a Path object, for example: target_path=Path('target')."
)

project_dir = Path(
updated_params.dbt_project.project_dir
if updated_params.dbt_project
Expand Down Expand Up @@ -717,7 +723,22 @@ def my_dbt_op(dbt: DbtCliResource):
if self._cli_version.major < 2:
try:
adapter = self._initialize_dbt_core_adapter(args)
except:
except ModuleNotFoundError as e:
if "dbt.adapters" in str(e):
logger.warning(
"A dbt adapter package could not be loaded. Please install the"
" appropriate dbt adapter for your data platform (for example:"
" dbt-postgres, dbt-bigquery, or dbt-snowflake). Some Dagster"
" features that require a live connection to your data platform"
" will be unavailable.\n\nOriginal error: %s",
e,
)
else:
logger.warning(
"An error was encountered when creating a handle to the dbt adapter in Dagster.",
exc_info=True,
)
Comment on lines +726 to +740

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.

P1 Double-nested error message produces confusing output

_initialize_dbt_core_adapter now raises a ModuleNotFoundError whose message already includes "Original error:\n{e}". When cli() then catches that exception and logs it as "...\n\nOriginal error: %s", e, the final warning prints the installation hint twice and "Original error" twice in nested form. A user would see something like:

A dbt adapter package could not be loaded … Original error: A dbt adapter package could not be found … Original error: No module named 'dbt.adapters.factory'

Additionally, the condition "dbt.adapters" in str(e) or "dbt-" in str(e) always evaluates to True for exceptions raised by _initialize_dbt_core_adapter, because the new custom message itself contains "dbt-postgres", "dbt-bigquery", and "dbt-snowflake". The else branch (lines 744–748) is effectively dead code on that specific error path, leaving the routing logic fragile—if the message in _initialize_dbt_core_adapter is ever changed, the routing silently breaks.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

except Exception:
logger.warning(
"An error was encountered when creating a handle to the dbt adapter in Dagster.",
exc_info=True,
Expand Down