3448 user defined default selection criteria #3875
Conversation
|
Closing and reopening to trigger adapter integration tests |
| for selector_name, selector in self.selectors.items(): | ||
| if selector["default"] is True: | ||
| name = selector_name | ||
| print_timestamped_line(f'Using default selector {name}') |
There was a problem hiding this comment.
Really happy to have logging that will help avoid user confusion. Two quick thoughts:
- [cosmetic] I think this should just be
logger.inforather thanprint_timestamped_line. It feels more like theFound 5 models, ...line (which doesn't include a timestamp), versus the actual execution steps that print with timestamps below it. - [functional] We don't want to log this line every time the user has a default selector defined, only if it's being used (i.e. no CLI
--selector--excludeargs). So I think this logging needs to go in theget_selection_spectask method instead, in the second conditional branch.
Since that's one more line of code to add in three places, I'm thinking: All of these tasks now use the same exact definition of get_selection_spec (thanks to #3554). What do you think of just updating the GraphRunnableTask method, so that instead of implementing as an abstractmethod it actually defines the logic that compile, list, and freshness will all use? Then we can delete the duplicated code from compile, list, and freshness, since they'll just inherit from GraphRunnableTask (just as run, test, etc inherit from compile).
There was a problem hiding this comment.
@jtcohen6 Do you know why list.py has spec = parse_difference(self.selector, ... in def get_selection_spec while freshness and compile have spec = parse_difference(self.args.select ?
There was a problem hiding this comment.
@TeddyCr Ah, it's just because list accepts both models and select. So options are:
- reimplement
get_selection_specforlistalone - implement
selectorselection_argas a property onGraphRunnableTask, and override just forListTask
I'm leaning toward the latter
There was a problem hiding this comment.
@TeddyCr Thanks for all the amazing work here! I'd really like to get this change in ahead of v0.21.0-rc1, so I'm going to merge this PR as soon as the tests pass. Then, I'll open a follow-on PR just to clean up the last bit of logging and duplicative task logic.
* Added selector default * Updated code from review feedback * Fix schema.yml location Co-authored-by: Jeremy Cohen <jeremy@fishtownanalytics.com> automatic commit by git-black, original commits: 4845174
resolves #3448
Description
This PR introduces a
defaultfield in theselectors.ymlallowing users to define 1 default selector that will execute the definition on eachdbt run,dbt compile,dbt list, ordbt source freshness.This is a clean PR of #3865 fixing git status after rebased perform to fix CLA error.
Checklist
CHANGELOG.mdand added information about my change to the "dbt next" section.