From de7cbc63ff826c00ccdaaae77e5393912b24e1d2 Mon Sep 17 00:00:00 2001 From: Matthieu Caneill Date: Tue, 5 Mar 2024 17:00:48 +0100 Subject: [PATCH 1/3] Create rule registry and discovery --- src/dbt_score/definitions.py | 16 ++++++++++ src/dbt_score/exceptions.py | 11 +++++++ src/dbt_score/registry.py | 56 ++++++++++++++++++++++++++++++++++ src/dbt_score/rules/example.py | 8 +++++ tests/rules/example.py | 9 ++++++ tests/rules/nested/__init__.py | 1 + tests/rules/nested/example.py | 9 ++++++ tests/test_rule_registry.py | 27 ++++++++++++++++ 8 files changed, 137 insertions(+) create mode 100644 src/dbt_score/definitions.py create mode 100644 src/dbt_score/exceptions.py create mode 100644 src/dbt_score/registry.py create mode 100644 src/dbt_score/rules/example.py create mode 100644 tests/rules/example.py create mode 100644 tests/rules/nested/__init__.py create mode 100644 tests/rules/nested/example.py create mode 100644 tests/test_rule_registry.py diff --git a/src/dbt_score/definitions.py b/src/dbt_score/definitions.py new file mode 100644 index 0000000..5465391 --- /dev/null +++ b/src/dbt_score/definitions.py @@ -0,0 +1,16 @@ +"""dbt-score definitions.""" + + +from functools import wraps +from typing import Any, Callable + + +def rule(func: Callable[..., None]) -> Callable[..., None]: + """Wrapper to create a rule.""" + + @wraps(func) + def wrapper(*args: Any, **kwargs: Any) -> Any: + return func(*args, **kwargs) + + wrapper._is_rule = True # type: ignore + return wrapper diff --git a/src/dbt_score/exceptions.py b/src/dbt_score/exceptions.py new file mode 100644 index 0000000..257b1f7 --- /dev/null +++ b/src/dbt_score/exceptions.py @@ -0,0 +1,11 @@ +"""dbt-score exceptions.""" + + +class DuplicatedRuleException(Exception): + """Two rules with the same name are defined.""" + + def __init__(self, rule_name: str): + """Instantiate exception.""" + super().__init__( + f"Rule {rule_name} is defined twice. Rules must have unique names." + ) diff --git a/src/dbt_score/registry.py b/src/dbt_score/registry.py new file mode 100644 index 0000000..7f405f2 --- /dev/null +++ b/src/dbt_score/registry.py @@ -0,0 +1,56 @@ +"""Rule registry. + +This module implements rule discovery. +""" + +import importlib +import pkgutil +from typing import Callable, Iterator + +from dbt_score.exceptions import DuplicatedRuleException + +THIRD_PARTY_RULES_NAMESPACE = "dbt_score_rules" + + +class RuleRegistry: + """A container for configured rules.""" + + def __init__(self) -> None: + """Instantiate a rule registry.""" + self._rules: dict[str, Callable[[], None]] = {} + + @property + def rules(self) -> dict[str, Callable[[], None]]: + """Get all rules.""" + return self._rules + + def _walk_packages(self, namespace_name: str) -> Iterator[str]: + """Walk packages and sub-packages recursively.""" + try: + namespace = importlib.import_module(namespace_name) + except ImportError: # no custom rule in Python path + return + + def onerror(module_name: str) -> None: + print(f"Failed to import {module_name}.") + + for package in pkgutil.walk_packages(namespace.__path__, onerror=onerror): + yield f"{namespace_name}.{package.name}" + if package.ispkg: + yield from self._walk_packages(f"{namespace_name}.{package.name}") + + def _load(self, namespace_name: str) -> None: + """Load rules found in a given namespace.""" + for module_name in self._walk_packages(namespace_name): + module = importlib.import_module(module_name) + for obj_name in dir(module): + obj = module.__dict__[obj_name] + if getattr(obj, "_is_rule", False): + if obj_name in self.rules: + raise DuplicatedRuleException(obj_name) + self._rules[obj_name] = obj + + def load_all(self) -> None: + """Load all rules, core and third-party.""" + self._load("dbt_score.rules") + self._load(THIRD_PARTY_RULES_NAMESPACE) diff --git a/src/dbt_score/rules/example.py b/src/dbt_score/rules/example.py new file mode 100644 index 0000000..fa7a7ba --- /dev/null +++ b/src/dbt_score/rules/example.py @@ -0,0 +1,8 @@ +"""Placeholder for example rules.""" + +from dbt_score.definitions import rule + + +@rule +def rule_example() -> None: + """An example rule.""" diff --git a/tests/rules/example.py b/tests/rules/example.py new file mode 100644 index 0000000..8ca1df3 --- /dev/null +++ b/tests/rules/example.py @@ -0,0 +1,9 @@ +"""Example rules.""" + + +from dbt_score.definitions import rule + + +@rule +def rule_test_example(): + """An example rule.""" diff --git a/tests/rules/nested/__init__.py b/tests/rules/nested/__init__.py new file mode 100644 index 0000000..009e1b1 --- /dev/null +++ b/tests/rules/nested/__init__.py @@ -0,0 +1 @@ +"""Nested package for testing rule discovery.""" diff --git a/tests/rules/nested/example.py b/tests/rules/nested/example.py new file mode 100644 index 0000000..78a67e8 --- /dev/null +++ b/tests/rules/nested/example.py @@ -0,0 +1,9 @@ +"""Example rules.""" + + +from dbt_score.definitions import rule + + +@rule +def rule_test_nested_example(): + """An example rule.""" diff --git a/tests/test_rule_registry.py b/tests/test_rule_registry.py new file mode 100644 index 0000000..4329a06 --- /dev/null +++ b/tests/test_rule_registry.py @@ -0,0 +1,27 @@ +"""Unit tests for the rule registry.""" + +import pytest +from dbt_score.exceptions import DuplicatedRuleException +from dbt_score.registry import RuleRegistry + + +def test_rule_registry_discovery(): + """Ensure rules can be found in a given namespace recursively.""" + r = RuleRegistry() + r._load("tests.rules") + assert sorted(r.rules.keys()) == ["rule_test_example", "rule_test_nested_example"] + + +def test_rule_registry_no_duplicates(): + """Ensure no duplicate rule names can coexist.""" + r = RuleRegistry() + r._load("tests.rules") + with pytest.raises(DuplicatedRuleException): + r._load("tests.rules") + + +def test_rule_registry_core_rules(): + """Ensure core rules are automatically discovered.""" + r = RuleRegistry() + r.load_all() + assert len(r.rules) > 0 From b40d04b9277b6f0e95d9c943d18e3d09d9df6c99 Mon Sep 17 00:00:00 2001 From: Matthieu Caneill Date: Thu, 28 Mar 2024 17:25:53 +0100 Subject: [PATCH 2/3] Adapt to changes in rule definition --- src/dbt_score/definitions.py | 16 ---------------- src/dbt_score/registry.py | 9 +++++---- src/dbt_score/rules/example.py | 8 -------- src/dbt_score/rules/generic.py | 1 + tests/rules/example.py | 8 ++++---- tests/rules/nested/example.py | 8 ++++---- 6 files changed, 14 insertions(+), 36 deletions(-) delete mode 100644 src/dbt_score/definitions.py delete mode 100644 src/dbt_score/rules/example.py diff --git a/src/dbt_score/definitions.py b/src/dbt_score/definitions.py deleted file mode 100644 index 5465391..0000000 --- a/src/dbt_score/definitions.py +++ /dev/null @@ -1,16 +0,0 @@ -"""dbt-score definitions.""" - - -from functools import wraps -from typing import Any, Callable - - -def rule(func: Callable[..., None]) -> Callable[..., None]: - """Wrapper to create a rule.""" - - @wraps(func) - def wrapper(*args: Any, **kwargs: Any) -> Any: - return func(*args, **kwargs) - - wrapper._is_rule = True # type: ignore - return wrapper diff --git a/src/dbt_score/registry.py b/src/dbt_score/registry.py index 7f405f2..f06621d 100644 --- a/src/dbt_score/registry.py +++ b/src/dbt_score/registry.py @@ -5,9 +5,10 @@ import importlib import pkgutil -from typing import Callable, Iterator +from typing import Iterator, Type from dbt_score.exceptions import DuplicatedRuleException +from dbt_score.rule import Rule THIRD_PARTY_RULES_NAMESPACE = "dbt_score_rules" @@ -17,10 +18,10 @@ class RuleRegistry: def __init__(self) -> None: """Instantiate a rule registry.""" - self._rules: dict[str, Callable[[], None]] = {} + self._rules: dict[str, Type[Rule]] = {} @property - def rules(self) -> dict[str, Callable[[], None]]: + def rules(self) -> dict[str, Type[Rule]]: """Get all rules.""" return self._rules @@ -45,7 +46,7 @@ def _load(self, namespace_name: str) -> None: module = importlib.import_module(module_name) for obj_name in dir(module): obj = module.__dict__[obj_name] - if getattr(obj, "_is_rule", False): + if type(obj) is type and issubclass(obj, Rule): if obj_name in self.rules: raise DuplicatedRuleException(obj_name) self._rules[obj_name] = obj diff --git a/src/dbt_score/rules/example.py b/src/dbt_score/rules/example.py deleted file mode 100644 index fa7a7ba..0000000 --- a/src/dbt_score/rules/example.py +++ /dev/null @@ -1,8 +0,0 @@ -"""Placeholder for example rules.""" - -from dbt_score.definitions import rule - - -@rule -def rule_example() -> None: - """An example rule.""" diff --git a/src/dbt_score/rules/generic.py b/src/dbt_score/rules/generic.py index 0a0c6d8..a6ccfca 100644 --- a/src/dbt_score/rules/generic.py +++ b/src/dbt_score/rules/generic.py @@ -1,4 +1,5 @@ """All generic rules.""" + from dbt_score.models import Model from dbt_score.rule import RuleViolation, rule diff --git a/tests/rules/example.py b/tests/rules/example.py index 8ca1df3..e9a9068 100644 --- a/tests/rules/example.py +++ b/tests/rules/example.py @@ -1,9 +1,9 @@ """Example rules.""" +from dbt_score.models import Model +from dbt_score.rule import RuleViolation, rule -from dbt_score.definitions import rule - -@rule -def rule_test_example(): +@rule() +def rule_test_example(model: Model) -> RuleViolation | None: """An example rule.""" diff --git a/tests/rules/nested/example.py b/tests/rules/nested/example.py index 78a67e8..4eccd8d 100644 --- a/tests/rules/nested/example.py +++ b/tests/rules/nested/example.py @@ -1,9 +1,9 @@ """Example rules.""" +from dbt_score.models import Model +from dbt_score.rule import RuleViolation, rule -from dbt_score.definitions import rule - -@rule -def rule_test_nested_example(): +@rule() +def rule_test_nested_example(model: Model) -> RuleViolation | None: """An example rule.""" From cfc015b465e30ece07d2369398f1644f646593f6 Mon Sep 17 00:00:00 2001 From: Matthieu Caneill Date: Thu, 28 Mar 2024 17:34:38 +0100 Subject: [PATCH 3/3] Rename module to rule_registry --- src/dbt_score/{registry.py => rule_registry.py} | 0 tests/test_rule_registry.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename src/dbt_score/{registry.py => rule_registry.py} (100%) diff --git a/src/dbt_score/registry.py b/src/dbt_score/rule_registry.py similarity index 100% rename from src/dbt_score/registry.py rename to src/dbt_score/rule_registry.py diff --git a/tests/test_rule_registry.py b/tests/test_rule_registry.py index 4329a06..0d90166 100644 --- a/tests/test_rule_registry.py +++ b/tests/test_rule_registry.py @@ -2,7 +2,7 @@ import pytest from dbt_score.exceptions import DuplicatedRuleException -from dbt_score.registry import RuleRegistry +from dbt_score.rule_registry import RuleRegistry def test_rule_registry_discovery():