Skip to content
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

Expand --greedy test selection to have a flag in profile.yml file #3997

Closed
isaacsantelli opened this issue Oct 4, 2021 · 1 comment · Fixed by #4270
Closed

Expand --greedy test selection to have a flag in profile.yml file #3997

isaacsantelli opened this issue Oct 4, 2021 · 1 comment · Fixed by #4270
Labels
enhancement New feature or request good_first_issue Straightforward + self-contained changes, good for new contributors!

Comments

@isaacsantelli
Copy link

isaacsantelli commented Oct 4, 2021

Describe the feature

Starting in 0.20.0 test selection was changed to be less greedy meaning that things like relationship tests which depend on more than one model are not selected by default on single model test runs. @joellabes has a great PR to enable a --greedy flag in 0.21.0. It would be a great if you could enable this feature the same way you could enable partial parse or other configuration options and make it the default.

Describe alternatives you've considered

The alternative, which is the current system, is to just have --greedy on each test run. There's nothing explicitly wrong with this but it is tedious.

Who will this benefit?

To my mind this will benefit a lot of dbt users; its always better imho to over test rather than undertest and since we have many tests that rely on multiple models, we prefer greedy test selection to less greedy selection. Of course this is a matter of preference though I know from seeing other people asking on slack that others feel similarly.

Are you interested in contributing this feature?

I've never contributed to a public repo actually but I love using dbt so if I could get some help getting started I'd be super excited to contribute!

@isaacsantelli isaacsantelli added enhancement New feature or request triage labels Oct 4, 2021
@jtcohen6 jtcohen6 added good_first_issue Straightforward + self-contained changes, good for new contributors! and removed triage labels Oct 7, 2021
@jtcohen6
Copy link
Contributor

jtcohen6 commented Oct 7, 2021

@isaac-taylor Thanks for the issue, and glad you're getting benefit out of the --greedy feature in v0.21! I hear you—it's a matter of preference, and also case-dependent. Looks like I've really managed to split the room on this one :)

I'd love to have your help with this one. The good news is, it's easier than ever before—in the lead-up to v1.0, we've significantly simplified the logic to working with global flags / user settings in dbt. There are two main changes you'd need to make:

  • Add greedy to UserConfig. This allows you to set it in the config section of profiles.yml.
  • In flags.py, move GREEDY into the group of "Global CLI commands." Add it to flag_defaults, and switch its entry in set_from_args to use get_flag_value. The cool thing is, the get_flag_value method will automatically check for an env var named DBT_GREEDY, too.
  • Finally, add a test to test_flags.py

If we're going to add greedy in more places, it does get me thinking that we should call it something more specific—perhaps greedy_selection. What do you think? This would either be a breaking change, or require a bit of extra logic to provide backwards compatibility, but it's worth getting the naming right sooner rather than later.


In the meantime, @joellabes and I are going to spend some time writing up "In defence of lazy expansion." I still stand by the switch, in v0.20, to less-greedy test selection, when you're only selecting a specific subset of project resources—but I think it's high time to provide a more thorough account of our rationale. In the meantime, it doesn't hurt to make it more configurable for users who disagree, or (better yet) who might want different behavior in dev vs. CI vs. prod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good_first_issue Straightforward + self-contained changes, good for new contributors!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants