From 8256171d9250d6902f7ab355cb6c1cf77d1e2f55 Mon Sep 17 00:00:00 2001 From: Jochem van Dooren Date: Wed, 20 Mar 2024 14:43:00 +0100 Subject: [PATCH] Remove non-default rules --- src/dbt_score/rules/example_rules.py | 97 ---------------------------- src/dbt_score/rules/rules.py | 28 ++++++++ tests/test_models.py | 10 ++- tests/test_rule.py | 7 +- 4 files changed, 40 insertions(+), 102 deletions(-) delete mode 100644 src/dbt_score/rules/example_rules.py create mode 100644 src/dbt_score/rules/rules.py diff --git a/src/dbt_score/rules/example_rules.py b/src/dbt_score/rules/example_rules.py deleted file mode 100644 index 9401c3c..0000000 --- a/src/dbt_score/rules/example_rules.py +++ /dev/null @@ -1,97 +0,0 @@ -"""All general rules.""" - -from dbt_score.models import Model -from dbt_score.rule import Rule, RuleViolation, rule - - -class ComplexRule(Rule): - """Complex rule.""" - - description = "Example of a complex rule." - - def preprocess(self) -> int: - """Preprocessing.""" - return len(self.description) - - def evaluate(self, model: Model) -> RuleViolation | None: - """Evaluate model.""" - x = self.preprocess() - - if x: - return RuleViolation(str(x)) - else: - return None - - -@rule() -def has_owner(model: Model) -> RuleViolation | None: - """A model should have an owner defined.""" - if "owner" not in model.meta: - return RuleViolation("Define the owner of the model in the meta section.") - - return None - - -@rule() -def has_primary_key(model: Model) -> RuleViolation | None: - """A model should have a primary key defined, unless it's a view.""" - if not model.config.get("materialized") == "picnic_view": - has_pk = False - for column in model.columns: - if "primary_key" in [constraint.type for constraint in column.constraints]: - has_pk = True - break - - if not has_pk: - return RuleViolation() - - return None - - -@rule() -def primary_key_has_uniqueness_test(model: Model) -> RuleViolation | None: - """Primary key columns should have a uniqueness test defined.""" - columns_with_pk = [] - if model.config.get("materialized") == "view": - for column in model.columns: - if "primary_key" in [constraint.type for constraint in column.constraints]: - columns_with_pk.append(column) - - tests = columns_with_pk[0].tests if len(columns_with_pk) == 1 else model.tests - - if columns_with_pk and "unique" not in [test.type for test in tests]: - return RuleViolation() - - return None - - -@rule() -def columns_have_description(model: Model) -> RuleViolation | None: - """All columns of a model should have a description.""" - invalid_column_names = [ - column.name for column in model.columns if not column.description - ] - if invalid_column_names: - return RuleViolation( - message=f"The following columns lack a description: " - f"{', '.join(invalid_column_names)}." - ) - - return None - - -@rule(description="A model should have at least one test defined.") -def has_test(model: Model) -> RuleViolation | None: - """A model should have at least one model-level or column-level test defined. - - This does not include singular tests, which are tests defined in a separate .sql - file and not linked to the model in the metadata. - """ - column_tests = [] - for column in model.columns: - column_tests.extend(column.tests) - - if len(model.tests) == 0 and len(column_tests) == 0: - return RuleViolation("Define a test for the model on model- or column-level.") - - return None diff --git a/src/dbt_score/rules/rules.py b/src/dbt_score/rules/rules.py new file mode 100644 index 0000000..48ba5fc --- /dev/null +++ b/src/dbt_score/rules/rules.py @@ -0,0 +1,28 @@ +"""All general rules.""" + +from dbt_score.models import Model +from dbt_score.rule import RuleViolation, rule + + +@rule() +def has_description(model: Model) -> RuleViolation | None: + """A model should have a description.""" + if not model.description: + return RuleViolation(message="Model lacks a description.") + + return None + + +@rule() +def columns_have_description(model: Model) -> RuleViolation | None: + """All columns of a model should have a description.""" + invalid_column_names = [ + column.name for column in model.columns if not column.description + ] + if invalid_column_names: + return RuleViolation( + message=f"The following columns lack a description: " + f"{', '.join(invalid_column_names)}." + ) + + return None diff --git a/tests/test_models.py b/tests/test_models.py index 645c9b4..e39046a 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -11,8 +11,12 @@ def test_manifest(read_text, raw_manifest): """Test loading a manifest.""" with patch("dbt_score.models.json.loads", return_value=raw_manifest): loader = ManifestLoader(Path("manifest.json")) - assert len(loader.models) == len([node for node in - raw_manifest["nodes"].values() - if node["resource_type"] == "model"]) + assert len(loader.models) == len( + [ + node + for node in raw_manifest["nodes"].values() + if node["resource_type"] == "model" + ] + ) assert loader.models[0].tests[0].name == "test2" assert loader.models[0].columns[0].tests[0].name == "test1" diff --git a/tests/test_rule.py b/tests/test_rule.py index 63901e8..d4ae41c 100644 --- a/tests/test_rule.py +++ b/tests/test_rule.py @@ -15,7 +15,8 @@ def assertions(rule_instance): assert rule_instance.severity == Severity.MEDIUM assert rule_instance.description == "Description of the rule." assert rule_instance.evaluate(model1) == RuleViolation( - message="Model1 is a violation.") + message="Model1 is a violation." + ) assert rule_instance.evaluate(model2) is None assertions(decorator_rule_instance) @@ -25,6 +26,7 @@ def assertions(rule_instance): def test_missing_description_rule_class(): """Test missing description in rule class.""" with pytest.raises(TypeError): + class BadRule(Rule): """Bad example rule.""" @@ -35,12 +37,13 @@ def evaluate(self, model: Model) -> RuleViolation | None: def test_missing_evaluate_rule_class(model1): """Test missing evaluate implementation in rule class.""" + class BadRule(Rule): """Bad example rule.""" + description = "Description of the rule." rule = BadRule() with pytest.raises(NotImplementedError): rule.evaluate(model1) -