new node selectors (#2425) #2629
Conversation
beckjake
force-pushed
the
feature/advanced-node-selection
branch
2 times, most recently
from
5362acf
to
6e161ab
Compare
beckjake
force-pushed
the
feature/new-node-selectors
branch
2 times, most recently
from
37c82d5
to
6a45b6a
Compare
- added a bunch of unit tests - added some new integration tests - cleaned up some selection logic
beckjake
force-pushed
the
feature/new-node-selectors
branch
from
6a45b6a
to
a523356
Compare
There was a problem hiding this comment.
@beckjake This is really cool. The config selector in particular opens up a lot of doors. I left a comment around some config-based selectors that I'd expected to return nodes and came up empty.
After several minutes of trying to get the CLI arg syntax right, I'm all the more excited about our plans to support YML-configured selectors.
One surprise: the new node selectors do not seem to work with dbt ls. I frequently wanted to check selection syntax by seeing the list of included nodes before actually running/testing. If that's especially tricky to implement or out of scope of this PR, I'll open a separate issue and prioritize it.
User error :) these work perfectly with dbt ls -s, I forgot that dbt ls -m can only return model nodes.
| def search( | ||
| self, included_nodes: Set[UniqueId], selector: str | ||
| ) -> Iterator[UniqueId]: | ||
| parts = self.arguments |
There was a problem hiding this comment.
This means all config arguments should be fair game for 'config.arg:value', right?
These worked for me on Fishtown's internal analytics project:
dbt run -m 'config.schema:analytics_utils'
dbt run -m 'config.materialized'
dbt test -m 'config.severity:error'
Whereas these did not match any nodes, when I believe they should have:
dbt run -m 'config.incremental_strategy:delete+insert'
dbt run -m 'config.unique_key:event_id'
There was a problem hiding this comment.
Oh, nice catch! That's because unique_key and incremental_strategy are special, so they don't show up to getattr. I'll change this to treat things like dictionaries when getattr fails.
There was a problem hiding this comment.
Nice, all work as expected
| def search( | ||
| self, included_nodes: Set[UniqueId], selector: str | ||
| ) -> Iterator[UniqueId]: | ||
| parts = self.arguments |
There was a problem hiding this comment.
Nice, all work as expected
|
@kwigley sorry about that, I did not mean to merge it without your review, I mis-clicked! I'll open a PR to revert this. |
|
@beckjake no worries! I got a chance to review this and don't have any meaningful feedback. Impl and tests looks great! no need to revert unless you think it is necessary |
resolves #2425
Description
Add the node selectors discussed in #2425, except I used
test_typefor differentiating schema and data tests, andtest_nameto differentiate between schema test names.There's obviously lots of room for new selectors in addition to these, but this felt like a nice start.
Checklist
CHANGELOG.mdand added information about my change to the "dbt next" section.