-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Provide ability to exclude resource_types, instead of listing everything not excluded #9756
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9756 +/- ##
==========================================
+ Coverage 88.07% 88.12% +0.04%
==========================================
Files 178 178
Lines 22445 22460 +15
==========================================
+ Hits 19768 19792 +24
+ Misses 2677 2668 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
core/dbt/task/clone.py
Outdated
|
||
return list(values) | ||
resource_types = [NodeType(rt) for rt in resource_types if rt in REFABLE_NODE_TYPES] |
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.
perhaps we could do this prior to returning from resource_types_from_args
so its not necessary here
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.
I think the point of this is not so much converting to NodeType (which I did move to "resource_types_from_args) but limiting it to REFABLE_NODE_TYPES. Otherwise somebody could specify a resource_type that's not actually clonable.
core/dbt/task/base.py
Outdated
|
||
|
||
def resource_types_from_args( | ||
args, all_resource_values: Set[str], default_resource_values: Set[str] |
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.
are all_resource_values and default_resource_values actually Set[NodeType]
s?
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.
Yes. Good catch. I've redefined them. As a sidenote, I'm always surprised by what mypy does and does not catch. I would have though it would complain about that. Maybe it automatically converts enums to strings?
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.
Looks great overall, the refactor is satisfying here! Just some comments on whether we can tighten up the typing on resource_types_from_args
Opened a new issue in dbt-labs/docs.getdbt.com: dbt-labs/docs.getdbt.com#5073 |
resolves #9237
Problem
People want to exclude unit_tests from running in the "build" command without specifying the complete list of all non-excluded resource types.
Solution
We've added a new cli param, --exclude-resource-types, also settable via the environment variable DBT_EXCLUDE_RESOURCE_TYPES. For consistency we've also added an environment variable for --resource-types, DBT_RESOURCE_TYPES.
Docs
We need to add some additional rows in the about-global-configs documentation page, for --resource-types/DBT_RESOURCE_TYPES and --exclude-resource-types/DBT_EXCLUDE_RESOURCE_TYPES. These params are used by the build, list, and clone commands.
Checklist