adding --selected-rule
#120
Conversation
There was a problem hiding this comment.
Super nice @ross-whatnot ! Thanks again for your contribution 🚀
I left a small comment, and two more things:
- Could you add a changelog entry?
- Add some docs to https://dbt-score.picnic.tech/configuration/ ?
| if manifest_provided and run_dbt_parse: | ||
| raise click.UsageError("--run-dbt-parse cannot be used with --manifest.") | ||
|
|
||
| if len(selected_rule) > 0 and len(disabled_rule) > 0: |
There was a problem hiding this comment.
Nice 👌
We also need to take into account pyproject.toml configuration I think.
Theoretically I could...
[tool.dbt-score]
disabled_rules = ["dbt_score.rules.generic.columns_have_description"]
selected_rules = ["dbt_score.rules.generic.model_has_description"]OR
[tool.dbt-score]
disabled_rules = ["dbt_score.rules.generic.columns_have_description"]dbt-score lint --selected-rule dbt_score.rules.generic.model_has_description
Which should not be possible IMO. So maybe we should add more validation to Config.validate()?
There was a problem hiding this comment.
Just catching up, it's been busy and lost track of this! I think your second example in particular feels like something I'd like to consider further. I could see utility in being able to say, effectively, dbt_score.rules.generic.columns_have_description is never run by default but there may be a certain scenario in which I explicitly want to run it, and it'd be beneficial to be able to do so at runtime.
Is it possible to override the TOML-level configs for disabled_rules from the command line? If dbt_score.rules.generic.columns_have_description is disabled per the TOML, can I un-disable it via the CLI in some way? It looks like I can potentially replace the list of rules that is disabled at runtime, but can I remove it?
There was a problem hiding this comment.
I believe that disabling 1 or more rule via the CLI (--disabled-rule foo --disabled-rule bar) will effectively replace the set of rules disabled via the toml config file.
Meaning you can remove at runtime the disabled rules from the config file, but only if you disable at least another rule 😅 Which is of course not great.
Not sure what the best approach is there. An example could be changing the CLI parameter to accept lists, including empty lists, e.g. --disabled-rules foo bar or --disabled-rules (no value = empty list).
| if rule_name in self._rules: | ||
| raise DuplicatedRuleException(rule_name) | ||
| if rule_name not in self.config.disabled_rules: | ||
| if ( |
There was a problem hiding this comment.
Could you add a small comment that explains this? Our future selves will be grateful 😂
|
|
||
|
|
||
| def test_selected_and_disabled_rule_registry_discovery(): | ||
| """Ensure selected rules are run even if otherwise disabled. |
There was a problem hiding this comment.
Nice I see it does work as expected, still I think my comment makes sense; shouldn't we warn the user about this invalid config? It will be less magical to the users I think
|
Hey @ross-whatnot, let me know what you want to do with this PR! If you have no time, I can pick it up from here if that suits you |
3 participants
As discussed in #118, adding a
--selected-ruleCLI flag to enable only specific rules to be run, either for testing during rules development or in specialized cases for wanting to run narrowly scoped rulesets as a part of CI.Intended behavior:
--selected-ruleCLI flag, which is mutually exclusive with--disabled-rule--selected-ruleis provided, the list of selected rules will be executed, even if they are disabled by pyproject.tomlAdded some tests, and manual validation confirms it appears to be working as expected.