-
Notifications
You must be signed in to change notification settings - Fork 358
Feat!: Introduce Model linting #3787
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
0a735d7
Feat: Introduce model linting
VaggelisD 52fdbdc
PR Feedback 12
VaggelisD 0f6250d
Add rule violations to OptimizedQueryCache
VaggelisD 8280209
Lift caching constraint for error violations
VaggelisD d3a377c
Refactor console setting to a decorator
VaggelisD a5a39d8
PR Feedback 13
VaggelisD 3394afa
Move violated_rules to QueryRenderer, formatting fixes
VaggelisD f1d7019
Bump migration script
VaggelisD File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,3 +20,8 @@ after_all: | |
|
|
||
| model_defaults: | ||
| dialect: 'duckdb' | ||
|
|
||
| linter: | ||
| enabled: True | ||
|
|
||
| warn_rules: "ALL" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| """Contains all the standard rules included with SQLMesh""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import typing as t | ||
|
|
||
| from sqlmesh.core.linter.rule import Rule, RuleViolation | ||
| from sqlmesh.core.model import Model | ||
|
|
||
|
|
||
| class NoMissingDescription(Rule): | ||
| """All models should be documented.""" | ||
|
|
||
| def check_model(self, model: Model) -> t.Optional[RuleViolation]: | ||
| return self.violation() if not model.description else None |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,3 +20,8 @@ after_all: | |
|
|
||
| model_defaults: | ||
| dialect: 'duckdb' | ||
|
|
||
| linter: | ||
| enabled: True | ||
|
|
||
| ignored_rules: "ALL" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import typing as t | ||
|
|
||
| from sqlmesh.core.linter.rule import Rule, RuleViolation | ||
| from sqlmesh.core.model import Model | ||
|
|
||
|
|
||
| class NoMissingOwner(Rule): | ||
| """All models should have an owner specified.""" | ||
|
|
||
| def check_model(self, model: Model) -> t.Optional[RuleViolation]: | ||
| return self.violation() if not model.owner else None |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import typing as t | ||
|
|
||
| from sqlglot import exp | ||
| from sqlglot.helper import ensure_collection | ||
|
|
||
| from sqlmesh.core.config.base import BaseConfig | ||
|
|
||
| from sqlmesh.utils.pydantic import field_validator | ||
|
|
||
|
|
||
| class LinterConfig(BaseConfig): | ||
| """Configuration for model linting | ||
|
|
||
| Args: | ||
| enabled: Flag indicating whether the linter should run | ||
|
|
||
| rules: A list of error rules to be applied on model | ||
| warn_rules: A list of rules to be applied on models but produce warnings instead of raising errors. | ||
| ignored_rules: A list of rules to be excluded/ignored | ||
|
|
||
| """ | ||
|
|
||
| enabled: bool = False | ||
|
|
||
| rules: t.Set[str] = set() | ||
| warn_rules: t.Set[str] = set() | ||
| ignored_rules: t.Set[str] = set() | ||
|
|
||
| @classmethod | ||
| def _validate_rules(cls, v: t.Any) -> t.Set[str]: | ||
| if isinstance(v, exp.Paren): | ||
| v = v.unnest().name | ||
| elif isinstance(v, (exp.Tuple, exp.Array)): | ||
| v = [e.name for e in v.expressions] | ||
| elif isinstance(v, exp.Expression): | ||
| v = v.name | ||
|
|
||
| return {name.lower() for name in ensure_collection(v)} | ||
|
|
||
| @field_validator("rules", "warn_rules", "ignored_rules", mode="before") | ||
| def rules_validator(cls, vs: t.Any) -> t.Set[str]: | ||
| return cls._validate_rules(vs) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import typing as t | ||
|
|
||
| from sqlmesh.core.config.linter import LinterConfig | ||
|
|
||
| from sqlmesh.core.model import Model | ||
|
|
||
| from sqlmesh.utils.errors import raise_config_error | ||
| from sqlmesh.core.console import get_console | ||
| from sqlmesh.core.linter.rule import RuleSet | ||
|
|
||
|
|
||
| def select_rules(all_rules: RuleSet, rule_names: t.Set[str]) -> RuleSet: | ||
| if "all" in rule_names: | ||
| return all_rules | ||
|
|
||
| rules = set() | ||
| for rule_name in rule_names: | ||
| if rule_name not in all_rules: | ||
| raise_config_error(f"Rule {rule_name} could not be found") | ||
|
|
||
| rules.add(all_rules[rule_name]) | ||
|
|
||
| return RuleSet(rules) | ||
|
|
||
|
|
||
| class Linter: | ||
| def __init__( | ||
| self, enabled: bool, all_rules: RuleSet, rules: RuleSet, warn_rules: RuleSet | ||
| ) -> None: | ||
| self.enabled = enabled | ||
| self.all_rules = all_rules | ||
VaggelisD marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| self.rules = rules | ||
| self.warn_rules = warn_rules | ||
|
|
||
| @classmethod | ||
| def from_rules(cls, all_rules: RuleSet, config: LinterConfig) -> Linter: | ||
| ignored_rules = select_rules(all_rules, config.ignored_rules) | ||
| included_rules = all_rules.difference(ignored_rules) | ||
|
|
||
| rules = select_rules(included_rules, config.rules) | ||
| warn_rules = select_rules(included_rules, config.warn_rules) | ||
|
|
||
| if overlapping := rules.intersection(warn_rules): | ||
| overlapping_rules = ", ".join(rule for rule in overlapping) | ||
| raise_config_error( | ||
| f"Rules cannot simultaneously warn and raise an error: [{overlapping_rules}]" | ||
| ) | ||
|
|
||
| return Linter(config.enabled, all_rules, rules, warn_rules) | ||
|
|
||
| def lint_model(self, model: Model) -> bool: | ||
| if not self.enabled: | ||
| return False | ||
|
|
||
| ignored_rules = select_rules(self.all_rules, model.ignored_rules) | ||
|
|
||
| rules = self.rules.difference(ignored_rules) | ||
| warn_rules = self.warn_rules.difference(ignored_rules) | ||
|
|
||
| error_violations = rules.check_model(model) | ||
| warn_violations = warn_rules.check_model(model) | ||
|
|
||
| if warn_violations: | ||
| get_console().show_linter_violations(warn_violations, model) | ||
|
|
||
| if error_violations: | ||
| get_console().show_linter_violations(error_violations, model, is_error=True) | ||
| return True | ||
|
|
||
| return False | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.