Skip to content

fix: allow null materialization in ModelAlertDataSchema for ClickHouse compatibility #2180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arbiv merged 1 commit into from
Apr 9, 2026
+5 −3
Merged

fix: allow null materialization in ModelAlertDataSchema for ClickHouse compatibility #2180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions elementary/monitor/alerts/model_alert.py
View file
Open in desktop
Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 concise_name not updated to handle None materialization, unlike summary

The summary property was updated (lines 113-115) to gracefully handle materialization=None, but the concise_name property (lines 104-109) was not. When materialization is None, concise_name falls through to the else branch and returns "dbt model alert - {alias}", which may be incorrect (e.g., if the resource is a snapshot whose materialization data is missing). The same pattern also exists at elementary/monitor/alerts/alert_messages/builder.py:195 (asset_type = "snapshot" if alert.materialization == "snapshot" else "model"), which was also not updated.

(Refers to lines 104-109)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional behavior. When materialization is None, falling through to the else branch and defaulting to "model" is the correct safe fallback. The same pattern at builder.py:195 also defaults to "model" for the same reason.

A snapshot whose materialization metadata is missing would be an unusual edge case — in practice, if a snapshot is correctly configured in dbt, its materialization field would be populated as "snapshot". The scenario triggering this bug is model alerts in ClickHouse where the materialization column is NULL due to missing metadata, and defaulting those to "model" is the appropriate behavior.

The summary property was updated differently because it interpolates materialization directly into the string (which would produce "dbt failed to build None "alias""), while concise_name only checks equality against "snapshot" and never renders the raw value.

Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ def __init__(
alias: str,
path: str,
original_path: str,
materialization: str,
full_refresh: bool,
alert_class_id: str,
materialization: Optional[str] = None,
message: Optional[str] = None,
model_unique_id: Optional[str] = None,
detected_at: Optional[datetime] = None,
Expand Down Expand Up @@ -110,7 +110,9 @@ def concise_name(self) -> str:

@property
def summary(self) -> str:
return f'dbt failed to build {self.materialization} "{self.alias}"'
if self.materialization:
return f'dbt failed to build {self.materialization} "{self.alias}"'
return f'dbt failed to build "{self.alias}"'

def get_report_link(self) -> Optional[ReportLinkData]:
return get_model_runs_link(self.report_url, self.model_unique_id)
2 changes: 1 addition & 1 deletion elementary/monitor/fetchers/alerts/schema/alert_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ class ModelAlertDataSchema(BaseAlertDataSchema):
alias: str
path: str
original_path: str
materialization: str
materialization: Optional[str] = None
full_refresh: bool
message: Optional[str] = None
resource_type: ResourceType = Field(ResourceType.MODEL, const=True) # type: ignore # noqa
Expand Down
Loading