Repository: PicnicSupermarket/dbt-score

Issue:
Configurable acceptable project score
Hi

It would be good if we could set either 

- a minimum project score 
- minimum required severity

and have `dbt-score` return a non-zero return code so that we can stop CI pipelines. At the minute, only the inability to find a manifest file returns a non-zero return code. Duplicate rules raise an exception

e.g. for minimum required project score 

```
[tool.dbt-score]
min_required_project_score=7.5
```

or passed on command line `--min_required_project_score 7.5`

for maximum allowed severity

```
[tool.dbt-score]
min_required_severity=MEDIUM
```
or passed on command line `--min_required_severity MEDIUM`

Should it raise an exception or simply log a message and exit with non-zero return code ?

Regards
Rob

Discussion:
WobblyRobbly: @jochemvandooren as we've added a Breaking Change i.e. will fail if the default model score isn't met (5.0) this should bump the version to 1.0.0 based on ?

```
## [1.0.0] - 2024-06-19

- Add `project_fail_under` configuration 
- Add `fail_any_model_under` configuration
- **breaking:** - default values of `5.0` for `project_fail_under` and `fail_any_model_under` will cause command to exit return code 1
```

matthieucan: > @jochemvandooren as we've added a Breaking Change i.e. will fail if the default model score isn't met (5.0) this should bump the version to 1.0.0 based on ?
> 
> ```
> ## [1.0.0] - 2024-06-19
> 
> - Add `project_fail_under` configuration 
> - Add `fail_any_model_under` configuration
> - **breaking:** - default values of `5.0` for `project_fail_under` and `fail_any_model_under` will cause command to exit return code 1
> ```

We're following semantic versioning, meaning if the major version is `0`, a breaking change will bump the minor version.
However no need for this to happen in the changelog file for this PR, this is done when releasing :)

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1ab3397..9f47610 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,13 @@ and this project adheres to
 
 ## [Unreleased]
 
+### Added
+
+- Add `project_fail_under` configuration
+- Add `fail_any_model_under` configuration
+- **Breaking:** - default values of `5.0` for `project_fail_under` and
+  `fail_any_model_under` will cause command to exit return code 1
+
 ## [0.2.1] - 2023-06-17
 
 - Lint the current dbt project only, not including the imported models.
diff --git a/docs/configuration.md b/docs/configuration.md
index 545be55..bbc051b 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -17,6 +17,8 @@ below:
 rule_namespaces = ["dbt_score.rules", "dbt_score_rules", "custom_rules"]
 disabled_rules = ["dbt_score.rules.generic.columns_have_description"]
 inject_cwd_in_python_path = true
+fail_project_under = 7.5
+fail_any_model_under = 8.0
 
 [tool.dbt-score.badges]
 first.threshold = 10.0
@@ -47,6 +49,10 @@ The following options can be set in the `pyproject.toml` file:
   this setting, that the default rules are in `dbt_score.rules` and are disabled
   if not included here.
 - `disabled_rules`: A list of rules to disable.
+- `fail_project_under` (default: `5.0`): If the project score is below this
+  value the command will fail with return code 1.
+- `fail_any_model_under` (default: `5.0`): If any model scores below this value
+  the command will fail with return code 1.
 
 #### Badges configuration
 
@@ -94,5 +100,5 @@ Many configuration options can also be set via the command line. To understand
 how to configure `dbt-score` from the command line:
 
 ```bash
-dbt score lint --help
+dbt-score lint --help
 ```
diff --git a/pyproject.toml b/pyproject.toml
index 788f499..4fe444f 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -90,13 +90,13 @@ extend-select = [
 ]
 
 [tool.ruff.lint.mccabe]
-max-complexity = 10
+max-complexity = 11
 
 [tool.ruff.lint.pydocstyle]
 convention = "google"
 
 [tool.ruff.lint.pylint]
-max-args = 8
+max-args = 9
 
 [tool.ruff.lint.per-file-ignores]
 "tests/**/*.py" = [
diff --git a/src/dbt_score/cli.py b/src/dbt_score/cli.py
index 6df65b4..9694b5e 100644
--- a/src/dbt_score/cli.py
+++ b/src/dbt_score/cli.py
@@ -78,6 +78,20 @@ def cli() -> None:
     is_flag=True,
     default=False,
 )
+@click.option(
+    "--fail_project_under",
+    help="Fail if the project score is under this value.",
+    type=float,
+    is_flag=False,
+    default=None,
+)
+@click.option(
+    "--fail_any_model_under",
+    help="Fail if any model is under this value.",
+    type=float,
+    is_flag=False,
+    default=None,
+)
 @click.pass_context
 def lint(
     ctx: click.Context,
@@ -87,6 +101,8 @@ def lint(
     disabled_rule: list[str],
     manifest: Path,
     run_dbt_parse: bool,
+    fail_project_under: float,
+    fail_any_model_under: float,
 ) -> None:
     """Lint dbt models metadata."""
     manifest_provided = (
@@ -102,14 +118,19 @@ def lint(
         config.overload({"rule_namespaces": namespace})
     if disabled_rule:
         config.overload({"disabled_rules": disabled_rule})
+    if fail_project_under:
+        config.overload({"fail_project_under": fail_project_under})
+    if fail_any_model_under:
+        config.overload({"fail_any_model_under": fail_any_model_under})
 
     if run_dbt_parse:
         dbt_parse()
 
     try:
-        lint_dbt_project(
+        evaluation = lint_dbt_project(
             manifest_path=manifest, config=config, format=format, select=select
         )
+
     except FileNotFoundError:
         logger.error(
             "dbt's manifest.json could not be found. If you're in a dbt project, be "
@@ -117,6 +138,12 @@ def lint(
         )
         ctx.exit(2)
 
+    if (
+        any(x.value < config.fail_any_model_under for x in evaluation.scores.values())
+        or evaluation.project_score.value < config.fail_project_under
+    ):
+        ctx.exit(1)
+
 
 @cli.command(name="list")
 @click.option(
diff --git a/src/dbt_score/config.py b/src/dbt_score/config.py
index e9eb118..4a4ddf5 100644
--- a/src/dbt_score/config.py
+++ b/src/dbt_score/config.py
@@ -55,6 +55,8 @@ class Config:
         "rule_namespaces",
         "disabled_rules",
         "inject_cwd_in_python_path",
+        "fail_project_under",
+        "fail_any_model_under",
     ]
     _rules_section: Final[str] = "rules"
     _badges_section: Final[str] = "badges"
@@ -67,6 +69,8 @@ class Config:
         self.rules_config: dict[str, RuleConfig] = {}
         self.config_file: Path | None = None
         self.badge_config: BadgeConfig = BadgeConfig()
+        self.fail_project_under: float = 5.0
+        self.fail_any_model_under: float = 5.0
 
     def set_option(self, option: str, value: Any) -> None:
         """Set an option in the config."""
diff --git a/src/dbt_score/formatters/__init__.py b/src/dbt_score/formatters/__init__.py
index 09466e1..ff37429 100644
--- a/src/dbt_score/formatters/__init__.py
+++ b/src/dbt_score/formatters/__init__.py
@@ -5,6 +5,7 @@ from __future__ import annotations
 import typing
 from abc import ABC, abstractmethod
 
+from dbt_score.config import Config
 from dbt_score.scoring import Score
 
 if typing.TYPE_CHECKING:
@@ -15,9 +16,10 @@ from dbt_score.models import ManifestLoader, Model
 class Formatter(ABC):
     """Abstract class to define a formatter."""
 
-    def __init__(self, manifest_loader: ManifestLoader):
+    def __init__(self, manifest_loader: ManifestLoader, config: Config):
         """Instantiate a formatter."""
         self._manifest_loader = manifest_loader
+        self._config = config
 
     @abstractmethod
     def model_evaluated(
diff --git a/src/dbt_score/formatters/human_readable_formatter.py b/src/dbt_score/formatters/human_readable_formatter.py
index 594b0ba..328a01e 100644
--- a/src/dbt_score/formatters/human_readable_formatter.py
+++ b/src/dbt_score/formatters/human_readable_formatter.py
@@ -1,6 +1,8 @@
 """Human readable formatter."""
 
 
+from typing import Any
+
 from dbt_score.evaluation import ModelResultsType
 from dbt_score.formatters import Formatter
 from dbt_score.models import Model
@@ -16,6 +18,11 @@ class HumanReadableFormatter(Formatter):
     label_warning = "\033[1;33mWARN\033[0m"
     label_error = "\033[1;31mERR \033[0m"
 
+    def __init__(self, *args: Any, **kwargs: Any):
+        """Instantiate formatter."""
+        super().__init__(*args, **kwargs)
+        self._failed_models: list[tuple[Model, Score]] = []
+
     @staticmethod
     def bold(text: str) -> str:
         """Return text in bold."""
@@ -25,6 +32,8 @@ class HumanReadableFormatter(Formatter):
         self, model: Model, results: ModelResultsType, score: Score
     ) -> None:
         """Callback when a model has been evaluated."""
+        if score.value < self._config.fail_any_model_under:
+            self._failed_models.append((model, score))
         print(
             f"{score.badge} {self.bold(model.name)} (score: {round(score.value, 1)!s})"
         )
@@ -43,3 +52,19 @@ class HumanReadableFormatter(Formatter):
     def project_evaluated(self, score: Score) -> None:
         """Callback when a project has been evaluated."""
         print(f"Project score: {self.bold(str(round(score.value, 1)))} {score.badge}")
+
+        if len(self._failed_models) > 0:
+            print()
+            print(
+                f"Error: model score too low, fail_any_model_under = "
+                f"{self._config.fail_any_model_under}"
+            )
+            for model, score in self._failed_models:
+                print(f"Model {model.name} scored {round(score.value, 1)}")
+
+        elif score.value < self._config.fail_project_under:
+            print()
+            print(
+                f"Error: project score too low, fail_project_under = "
+                f"{self._config.fail_project_under}"
+            )
diff --git a/src/dbt_score/lint.py b/src/dbt_score/lint.py
index 28ff0c0..5b6d95f 100644
--- a/src/dbt_score/lint.py
+++ b/src/dbt_score/lint.py
@@ -18,7 +18,7 @@ def lint_dbt_project(
     config: Config,
     format: Literal["plain", "manifest", "ascii"],
     select: Iterable[str] | None = None,
-) -> None:
+) -> Evaluation:
     """Lint dbt manifest."""
     if not manifest_path.exists():
         raise FileNotFoundError(f"Manifest not found at {manifest_path}.")
@@ -33,7 +33,7 @@ def lint_dbt_project(
         "manifest": ManifestFormatter,
         "ascii": ASCIIFormatter,
     }
-    formatter = formatters[format](manifest_loader=manifest_loader)
+    formatter = formatters[format](manifest_loader=manifest_loader, config=config)
 
     scorer = Scorer(config)
 
@@ -44,3 +44,5 @@ def lint_dbt_project(
         scorer=scorer,
     )
     evaluation.evaluate()
+
+    return evaluation


Test patch:
diff --git a/tests/formatters/test_ascii_formatter.py b/tests/formatters/test_ascii_formatter.py
index a53c9ca..f76cabb 100644
--- a/tests/formatters/test_ascii_formatter.py
+++ b/tests/formatters/test_ascii_formatter.py
@@ -9,6 +9,7 @@ from dbt_score.scoring import Score
 
 def test_ascii_formatter_model(
     capsys,
+    default_config,
     manifest_loader,
     model1,
     rule_severity_low,
@@ -16,7 +17,7 @@ def test_ascii_formatter_model(
     rule_severity_critical,
 ):
     """Ensure the formatter doesn't write anything after model evaluation."""
-    formatter = ASCIIFormatter(manifest_loader=manifest_loader)
+    formatter = ASCIIFormatter(manifest_loader=manifest_loader, config=default_config)
     results: dict[Type[Rule], RuleViolation | Exception | None] = {
         rule_severity_low: None,
         rule_severity_medium: Exception("Oh noes"),
@@ -27,9 +28,9 @@ def test_ascii_formatter_model(
     assert stdout == ""
 
 
-def test_ascii_formatter_project(capsys, manifest_loader):
+def test_ascii_formatter_project(capsys, default_config, manifest_loader):
     """Ensure the formatter has the correct output after project evaluation."""
-    formatter = ASCIIFormatter(manifest_loader=manifest_loader)
+    formatter = ASCIIFormatter(manifest_loader=manifest_loader, config=default_config)
 
     formatter.project_evaluated(Score(10.0, "🥇"))
     stdout_gold = capsys.readouterr().out
diff --git a/tests/formatters/test_human_readable_formatter.py b/tests/formatters/test_human_readable_formatter.py
index f86ee7c..295e804 100644
--- a/tests/formatters/test_human_readable_formatter.py
+++ b/tests/formatters/test_human_readable_formatter.py
@@ -9,6 +9,7 @@ from dbt_score.scoring import Score
 
 def test_human_readable_formatter_model(
     capsys,
+    default_config,
     manifest_loader,
     model1,
     rule_severity_low,
@@ -16,7 +17,9 @@ def test_human_readable_formatter_model(
     rule_severity_critical,
 ):
     """Ensure the formatter has the correct output after model evaluation."""
-    formatter = HumanReadableFormatter(manifest_loader=manifest_loader)
+    formatter = HumanReadableFormatter(
+        manifest_loader=manifest_loader, config=default_config
+    )
     results: dict[Type[Rule], RuleViolation | Exception | None] = {
         rule_severity_low: None,
         rule_severity_medium: Exception("Oh noes"),
@@ -35,9 +38,72 @@ def test_human_readable_formatter_model(
     )
 
 
-def test_human_readable_formatter_project(capsys, manifest_loader):
+def test_human_readable_formatter_project(capsys, default_config, manifest_loader):
     """Ensure the formatter has the correct output after project evaluation."""
-    formatter = HumanReadableFormatter(manifest_loader=manifest_loader)
+    formatter = HumanReadableFormatter(
+        manifest_loader=manifest_loader, config=default_config
+    )
     formatter.project_evaluated(Score(10.0, "🥇"))
     stdout = capsys.readouterr().out
     assert stdout == "Project score: \x1B[1m10.0\x1B[0m 🥇\n"
+
+
+def test_human_readable_formatter_low_model_score(
+    capsys,
+    default_config,
+    manifest_loader,
+    model1,
+    rule_severity_critical,
+):
+    """Ensure the formatter has the correct output when a model has a low score."""
+    formatter = HumanReadableFormatter(
+        manifest_loader=manifest_loader, config=default_config
+    )
+    results: dict[Type[Rule], RuleViolation | Exception | None] = {
+        rule_severity_critical: RuleViolation("Error"),
+    }
+    formatter.model_evaluated(model1, results, Score(0.0, "🚧"))
+    formatter.project_evaluated(Score(0.0, "🚧"))
+    stdout = capsys.readouterr().out
+    print()
+    assert (
+        stdout
+        == """🚧 \x1B[1mmodel1\x1B[0m (score: 0.0)
+    \x1B[1;33mWARN\x1B[0m (critical) tests.conftest.rule_severity_critical: Error
+
+Project score: \x1B[1m0.0\x1B[0m 🚧
+
+Error: model score too low, fail_any_model_under = 5.0
+Model model1 scored 0.0
+"""
+    )
+
+
+def test_human_readable_formatter_low_project_score(
+    capsys,
+    default_config,
+    manifest_loader,
+    model1,
+    rule_severity_critical,
+):
+    """Ensure the formatter has the correct output when the projet has a low score."""
+    formatter = HumanReadableFormatter(
+        manifest_loader=manifest_loader, config=default_config
+    )
+    results: dict[Type[Rule], RuleViolation | Exception | None] = {
+        rule_severity_critical: RuleViolation("Error"),
+    }
+    formatter.model_evaluated(model1, results, Score(10.0, "🥇"))
+    formatter.project_evaluated(Score(0.0, "🚧"))
+    stdout = capsys.readouterr().out
+    print()
+    assert (
+        stdout
+        == """🥇 \x1B[1mmodel1\x1B[0m (score: 10.0)
+    \x1B[1;33mWARN\x1B[0m (critical) tests.conftest.rule_severity_critical: Error
+
+Project score: \x1B[1m0.0\x1B[0m 🚧
+
+Error: project score too low, fail_project_under = 5.0
+"""
+    )
diff --git a/tests/formatters/test_manifest_formatter.py b/tests/formatters/test_manifest_formatter.py
index fa2bd0f..56c7daa 100644
--- a/tests/formatters/test_manifest_formatter.py
+++ b/tests/formatters/test_manifest_formatter.py
@@ -10,6 +10,7 @@ from dbt_score.scoring import Score
 
 def test_manifest_formatter_model(
     capsys,
+    default_config,
     manifest_loader,
     model1,
     rule_severity_low,
@@ -17,7 +18,9 @@ def test_manifest_formatter_model(
     rule_severity_critical,
 ):
     """Ensure the formatter has the correct output after model evaluation."""
-    formatter = ManifestFormatter(manifest_loader=manifest_loader)
+    formatter = ManifestFormatter(
+        manifest_loader=manifest_loader, config=default_config
+    )
     results = {
         rule_severity_low: None,
         rule_severity_medium: Exception("Oh noes"),
@@ -30,6 +33,7 @@ def test_manifest_formatter_model(
 
 def test_manifest_formatter_project(
     capsys,
+    default_config,
     manifest_loader,
     model1,
     model2,
@@ -38,7 +42,9 @@ def test_manifest_formatter_project(
     rule_severity_critical,
 ):
     """Ensure the formatter has the correct output after project evaluation."""
-    formatter = ManifestFormatter(manifest_loader=manifest_loader)
+    formatter = ManifestFormatter(
+        manifest_loader=manifest_loader, config=default_config
+    )
     result1: dict[Type[Rule], RuleViolation | Exception | None] = {
         rule_severity_low: None,
         rule_severity_medium: Exception("Oh noes"),
diff --git a/tests/resources/pyproject.toml b/tests/resources/pyproject.toml
index b317d90..eb598f3 100644
--- a/tests/resources/pyproject.toml
+++ b/tests/resources/pyproject.toml
@@ -1,6 +1,8 @@
 [tool.dbt-score]
 rule_namespaces = ["foo", "tests"]
 disabled_rules = ["foo.foo", "tests.bar"]
+fail_project_under = 7.5
+fail_any_model_under = 6.9
 
 [tool.dbt-score.badges]
 wip.icon = "🏗️"
diff --git a/tests/test_cli.py b/tests/test_cli.py
index cb50f19..68caa40 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -45,3 +45,31 @@ def test_lint_non_existing_manifest(caplog):
 
         assert result.exit_code == 2
         assert "dbt's manifest.json could not be found" in caplog.text
+
+
+def test_fail_project_under(manifest_path):
+    """Test `fail_project_under`."""
+    with patch("dbt_score.cli.Config._load_toml_file"):
+        runner = CliRunner()
+        result = runner.invoke(
+            lint, ["--manifest", manifest_path, "--fail_project_under", "10.0"]
+        )
+
+        assert "model1" in result.output
+        assert "model2" in result.output
+        assert "Error: project score too low, fail_project_under" in result.stdout
+        assert result.exit_code == 1
+
+
+def test_fail_any_model_under(manifest_path):
+    """Test `fail_any_model_under`."""
+    with patch("dbt_score.cli.Config._load_toml_file"):
+        runner = CliRunner()
+        result = runner.invoke(
+            lint, ["--manifest", manifest_path, "--fail_any_model_under", "10.0"]
+        )
+
+        assert "model1" in result.output
+        assert "model2" in result.output
+        assert "Error: model score too low, fail_any_model_under" in result.stdout
+        assert result.exit_code == 1
diff --git a/tests/test_config.py b/tests/test_config.py
index d70ad73..14c8392 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -25,6 +25,8 @@ def test_load_valid_toml_file(valid_config_path):
     assert config.badge_config.third.icon == "3️⃣"
     assert config.badge_config.second.icon == "2️⃣"
     assert config.badge_config.first.icon == "1️⃣"
+    assert config.fail_project_under == 7.5
+    assert config.fail_any_model_under == 6.9
 
 
 def test_load_invalid_toml_file(caplog, invalid_config_path):
diff --git a/tests/test_evaluation.py b/tests/test_evaluation.py
index ae5cac1..a7e7a4e 100644
--- a/tests/test_evaluation.py
+++ b/tests/test_evaluation.py
@@ -7,6 +7,7 @@ from dbt_score.evaluation import Evaluation
 from dbt_score.models import ManifestLoader
 from dbt_score.rule import RuleViolation
 from dbt_score.rule_registry import RuleRegistry
+from dbt_score.scoring import Score
 
 
 def test_evaluation_low_medium_high(
@@ -28,6 +29,9 @@ def test_evaluation_low_medium_high(
     rule_registry._add_rule(rule_severity_high)
     rule_registry._add_rule(rule_error)
 
+    # Ensure we get a valid Score object from the Mock
+    mock_scorer.score_model.return_value = Score(10, "🥇")
+
     evaluation = Evaluation(
         rule_registry=rule_registry,
         manifest_loader=manifest_loader,
@@ -66,12 +70,17 @@ def test_evaluation_critical(
     rule_registry._add_rule(rule_severity_low)
     rule_registry._add_rule(rule_severity_critical)
 
+    mock_formatter = Mock()
+    mock_scorer = Mock()
+    mock_scorer.score_model.return_value = Score(10, "🥇")
+
     evaluation = Evaluation(
         rule_registry=rule_registry,
         manifest_loader=manifest_loader,
-        formatter=Mock(),
-        scorer=Mock(),
+        formatter=mock_formatter,
+        scorer=mock_scorer,
     )
+
     evaluation.evaluate()
 
     model2 = manifest_loader.models[1]
@@ -85,11 +94,15 @@ def test_evaluation_no_rule(manifest_path, default_config):
 
     rule_registry = RuleRegistry(default_config)
 
+    mock_formatter = Mock()
+    mock_scorer = Mock()
+    mock_scorer.score_model.return_value = Score(10, "🥇")
+
     evaluation = Evaluation(
         rule_registry=rule_registry,
         manifest_loader=manifest_loader,
-        formatter=Mock(),
-        scorer=Mock(),
+        formatter=mock_formatter,
+        scorer=mock_scorer,
     )
     evaluation.evaluate()
 
@@ -148,11 +161,15 @@ def test_evaluation_rule_with_config(
     rule_registry = RuleRegistry(config)
     rule_registry._add_rule(rule_with_config)
 
+    mock_formatter = Mock()
+    mock_scorer = Mock()
+    mock_scorer.score_model.return_value = Score(10, "🥇")
+
     evaluation = Evaluation(
         rule_registry=rule_registry,
         manifest_loader=manifest_loader,
-        formatter=Mock(),
-        scorer=Mock(),
+        formatter=mock_formatter,
+        scorer=mock_scorer,
     )
     evaluation.evaluate()