-
Notifications
You must be signed in to change notification settings - Fork 220
Add support for upstream/downstream model selectors with graph operators #2056
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.
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
d21f10f
5a86050
be8d198
cf4dd43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -265,3 +265,65 @@ def dbt_runner_with_models_mock() -> Generator[MagicMock, None, None]: | |
| ) as mock_ls: | ||
| mock_ls.return_value = ["node_name_1", "node_name_2"] | ||
| yield mock_ls | ||
|
|
||
|
|
||
| def test_parse_selector_with_graph_operators_downstream( | ||
| dbt_runner_with_models_mock, anonymous_tracking_mock | ||
| ): | ||
| config = MockConfig("mock_project_dir") | ||
|
|
||
| data_monitoring_filter = SelectorFilter( | ||
| tracking=anonymous_tracking_mock, | ||
| config=config, | ||
| selector="model:customers+", | ||
| ) | ||
|
|
||
| assert data_monitoring_filter.get_filter().node_names == [ | ||
| "node_name_1", | ||
| "node_name_2", | ||
| ] | ||
| assert data_monitoring_filter.get_filter().selector == "model:customers+" | ||
|
|
||
|
|
||
| def test_parse_selector_with_graph_operators_upstream( | ||
| dbt_runner_with_models_mock, anonymous_tracking_mock | ||
| ): | ||
| config = MockConfig("mock_project_dir") | ||
|
|
||
| data_monitoring_filter = SelectorFilter( | ||
| tracking=anonymous_tracking_mock, | ||
| config=config, | ||
| selector="model:+customers", | ||
| ) | ||
|
|
||
| assert data_monitoring_filter.get_filter().node_names == [ | ||
| "node_name_1", | ||
| "node_name_2", | ||
| ] | ||
| assert data_monitoring_filter.get_filter().selector == "model:+customers" | ||
|
|
||
|
|
||
| def test_parse_selector_with_graph_operators_both( | ||
| dbt_runner_with_models_mock, anonymous_tracking_mock | ||
| ): | ||
| config = MockConfig("mock_project_dir") | ||
|
|
||
| data_monitoring_filter = SelectorFilter( | ||
| tracking=anonymous_tracking_mock, | ||
| config=config, | ||
| selector="model:+customers+", | ||
| ) | ||
|
|
||
| assert data_monitoring_filter.get_filter().node_names == [ | ||
| "node_name_1", | ||
| "node_name_2", | ||
| ] | ||
| assert data_monitoring_filter.get_filter().selector == "model:+customers+" | ||
|
|
||
|
|
||
|
Comment on lines
+270
to
+323
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mark dbt_runner_with_models_mock as used to satisfy Ruff (ARG001) In the three graph-operator tests, One simple pattern is to reference the argument at the top of each test: -def test_parse_selector_with_graph_operators_downstream(
- dbt_runner_with_models_mock, anonymous_tracking_mock
-):
+def test_parse_selector_with_graph_operators_downstream(
+ dbt_runner_with_models_mock, anonymous_tracking_mock
+):
+ _ = dbt_runner_with_models_mock # ensure fixture is used for linters
@@
-def test_parse_selector_with_graph_operators_upstream(
- dbt_runner_with_models_mock, anonymous_tracking_mock
-):
+def test_parse_selector_with_graph_operators_upstream(
+ dbt_runner_with_models_mock, anonymous_tracking_mock
+):
+ _ = dbt_runner_with_models_mock # ensure fixture is used for linters
@@
-def test_parse_selector_with_graph_operators_both(
- dbt_runner_with_models_mock, anonymous_tracking_mock
-):
+def test_parse_selector_with_graph_operators_both(
+ dbt_runner_with_models_mock, anonymous_tracking_mock
+):
+ _ = dbt_runner_with_models_mock # ensure fixture is used for lintersAlternatively, you could rename the fixture parameter to 🧰 Tools🪛 Ruff (0.14.5)271-271: Unused function argument: (ARG001) 289-289: Unused function argument: (ARG001) 307-307: Unused function argument: (ARG001) 🤖 Prompt for AI Agents |
||
| def test_has_graph_operators(): | ||
| assert SelectorFilter._has_graph_operators("customers+") is True | ||
| assert SelectorFilter._has_graph_operators("+customers") is True | ||
| assert SelectorFilter._has_graph_operators("+customers+") is True | ||
| assert SelectorFilter._has_graph_operators("customers") is False | ||
| assert SelectorFilter._has_graph_operators("my_model") is False | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Graph operator model filter silently dropped when dbt ls returns no matching nodes
In
from_cli_params, when amodels:filter with graph operators (e.g.,models:customers+) is resolved viaSelectorFilteranddbt lsreturns an empty list, the filter is silently dropped. Atelementary/monitor/data_monitoring/schema.py:247,if filter_result.node_names:is falsy for an empty list, so nothing is added to eithernode_namesormodels. Thecontinueon line 251 then skips to the next filter. This means a user who specifies--filters models:customers+where no nodes match the graph selector will see all alerts instead of no alerts, because the filter was effectively removed rather than applied as an empty match set.Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.