-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix issues with inline nodes and selectors #10264
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
cfbe8f7
Test for compile --inline with default selector
gshank f2f4fe7
Check for args.inline in get_selector_spec
gshank 4c71dae
Check for mutually_exclusive --select and --inline
gshank b91f848
Move tests to graph_selection. Add --selector to mutually_exclusive
gshank 35036e5
Hack to move inline node from disabled
gshank 0eca7e3
Changie
gshank 33526a9
Fix cli/flags.py
gshank ac20bde
Formatting
gshank File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
kind: Fixes | ||
body: Fix issues with selectors and inline nodes | ||
time: 2024-06-05T11:16:52.187667-04:00 | ||
custom: | ||
Author: gshank | ||
Issue: 8943 9269 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
import pytest | ||
|
||
from dbt.cli.exceptions import DbtUsageException | ||
from dbt.tests.util import run_dbt, run_dbt_and_capture, write_file | ||
|
||
selectors_yml = """ | ||
selectors: | ||
- name: test_selector | ||
description: Exclude everything | ||
default: true | ||
definition: | ||
method: package | ||
value: "foo" | ||
""" | ||
|
||
dbt_project_yml = """ | ||
name: test | ||
profile: test | ||
flags: | ||
send_anonymous_usage_stats: false | ||
""" | ||
|
||
dbt_project_yml_disabled_models = """ | ||
name: test | ||
profile: test | ||
flags: | ||
send_anonymous_usage_stats: false | ||
models: | ||
+enabled: false | ||
""" | ||
|
||
|
||
class TestCompileInlineWithSelector: | ||
@pytest.fixture(scope="class") | ||
def models(self): | ||
return { | ||
"first_model.sql": "select 1 as id", | ||
} | ||
|
||
@pytest.fixture(scope="class") | ||
def selectors(self): | ||
return selectors_yml | ||
|
||
def test_inline_selectors(self, project): | ||
(results, log_output) = run_dbt_and_capture( | ||
["compile", "--inline", "select * from {{ ref('first_model') }}"] | ||
) | ||
assert len(results) == 1 | ||
assert "Compiled inline node is:" in log_output | ||
|
||
# Set all models to disabled, check that we still get inline result | ||
write_file(dbt_project_yml_disabled_models, project.project_root, "dbt_project.yml") | ||
(results, log_output) = run_dbt_and_capture(["compile", "--inline", "select 1 as id"]) | ||
assert len(results) == 1 | ||
|
||
# put back non-disabled dbt_project and check for mutually exclusive error message | ||
# for --select and --inline | ||
write_file(dbt_project_yml, project.project_root, "dbt_project.yml") | ||
with pytest.raises(DbtUsageException): | ||
run_dbt(["compile", "--select", "first_model", "--inline", "select 1 as id"]) | ||
|
||
# check for mutually exclusive --selector and --inline | ||
with pytest.raises(DbtUsageException): | ||
run_dbt(["compile", "--selector", "test_selector", "--inline", "select 1 as id"]) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
So if all models are disabled, we effectively re-enable all of them? I think I'm missing something here as to why this would be safe to do, and how we know at this point that all models have been disabled
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.
The only way that an inline node would be disabled is if somebody has all models to enabled: false (like in the test case) because there's no place else for enabled: false to come from. This just enables and moves the temporary inline node that was just created. It doesn't affect anything else in the manifest.