Feat: Add CICD bot linter logging

VaggelisD · VaggelisD · commit 2b551ed97352 · 2025-03-25T11:39:08.000-04:00
diff --git a/sqlmesh/core/console.py b/sqlmesh/core/console.py
@@ -2151,7 +2151,6 @@ def _show_categorized_snapshots(self, plan: Plan, default_catalog: t.Optional[st
     def log_test_results(
         self, result: unittest.result.TestResult, output: t.Optional[str], target_dialect: str
     ) -> None:
-        # import ipywidgets as widgets
         if result.wasSuccessful():
             self._print(
                 f"**Successfully Ran `{str(result.testsRun)}` Tests Against `{target_dialect}`**\n\n"
@@ -2181,6 +2180,17 @@ def log_failed_models(self, errors: t.List[NodeExecutionFailedError]) -> None:
 
             self._print("```\n")
 
+    def show_linter_violations(
+        self, violations: t.List[RuleViolation], model: Model, is_error: bool = False
+    ) -> None:
+        severity = "errors" if is_error else "warnings"
+        violations_msg = "\n".join(f" - {violation}" for violation in violations)
+        msg = f"\nLinter {severity} for `{model._path}`:\n{violations_msg}\n"
+
+        self._print(msg)
+        if is_error:
+            self._errors.append(msg)
+
     def log_error(self, message: str) -> None:
         super().log_error(f"```\n\\[ERROR] {message}```\n\n")
 
diff --git a/sqlmesh/integrations/github/cicd/command.py b/sqlmesh/integrations/github/cicd/command.py
@@ -13,7 +13,7 @@
     GithubController,
     TestFailure,
 )
-from sqlmesh.utils.errors import CICDBotError, ConflictingPlanError, PlanError
+from sqlmesh.utils.errors import CICDBotError, ConflictingPlanError, PlanError, LinterError
 
 logger = logging.getLogger(__name__)
 
@@ -189,6 +189,16 @@ def deploy_production(ctx: click.Context) -> None:
 
 def _run_all(controller: GithubController) -> None:
     has_required_approval = False
+
+    controller.update_linter_check(status=GithubCheckStatus.QUEUED)
+
+    lint_error = controller._linter_errors is not None
+
+    controller.update_linter_check(
+        status=GithubCheckStatus.COMPLETED,
+        conclusion=GithubCheckConclusion.FAILURE if lint_error else GithubCheckConclusion.SUCCESS,
+    )
+
     is_auto_deploying_prod = (
         controller.deploy_command_enabled or controller.do_required_approval_check
     )
@@ -209,7 +219,7 @@ def _run_all(controller: GithubController) -> None:
     controller.update_test_check(status=GithubCheckStatus.QUEUED)
     if is_auto_deploying_prod:
         controller.update_prod_environment_check(status=GithubCheckStatus.QUEUED)
-    tests_passed = _run_tests(controller)
+    tests_passed = _run_tests(controller) if not lint_error else False
     if controller.do_required_approval_check:
         if has_required_approval:
             controller.update_required_approval_check(
@@ -219,22 +229,38 @@ def _run_all(controller: GithubController) -> None:
             controller.update_required_approval_check(status=GithubCheckStatus.QUEUED)
             has_required_approval = _check_required_approvers(controller)
     if not tests_passed:
+        if lint_error:
+            summary = "Linting"
+            exception_error = "Linter failed"
+        else:
+            summary = "Unit Test(s)"
+            exception_error = "Failed to run tests"
+
         controller.update_pr_environment_check(
             status=GithubCheckStatus.COMPLETED,
-            exception=TestFailure(),
+            exception=LinterError("") if lint_error else TestFailure(),
         )
         controller.update_prod_plan_preview_check(
             status=GithubCheckStatus.COMPLETED,
             conclusion=GithubCheckConclusion.SKIPPED,
-            summary="Unit Test(s) Failed so skipping creating prod plan",
+            summary=f"{summary} Failed so skipping creating prod plan",
         )
         if is_auto_deploying_prod:
             controller.update_prod_environment_check(
                 status=GithubCheckStatus.COMPLETED,
                 conclusion=GithubCheckConclusion.SKIPPED,
-                skip_reason="Unit Test(s) Failed so skipping deploying to production",
+                skip_reason=f"{summary} Failed so skipping deploying to production",
             )
-        raise CICDBotError("Failed to run tests. See check status for more information.")
+
+        if lint_error:
+            controller.update_test_check(
+                status=GithubCheckStatus.COMPLETED,
+                conclusion=GithubCheckConclusion.SKIPPED,
+                output="Linting failed, skipping tests",
+            )
+
+        raise CICDBotError(f"{exception_error}. See check status for more information.")
+
     pr_environment_updated = _update_pr_environment(controller)
     prod_plan_generated = False
     if pr_environment_updated:
diff --git a/sqlmesh/integrations/github/cicd/controller.py b/sqlmesh/integrations/github/cicd/controller.py
@@ -29,6 +29,7 @@
     format_intervals,
 )
 from sqlmesh.core.user import User
+from sqlmesh.core.config import Config
 from sqlmesh.integrations.github.cicd.config import GithubCICDBotConfig
 from sqlmesh.utils import word_characters_only
 from sqlmesh.utils.concurrency import NodeExecutionFailedError
@@ -38,6 +39,7 @@
     NoChangesPlanError,
     PlanError,
     UncategorizedPlanError,
+    LinterError,
 )
 from sqlmesh.utils.pydantic import PydanticModel
 
@@ -50,8 +52,6 @@
     from github.PullRequestReview import PullRequestReview
     from github.Repository import Repository
 
-    from sqlmesh.core.config import Config
-
 logger = logging.getLogger(__name__)
 
 
@@ -302,6 +302,7 @@ def __init__(
         self._prod_plan_builder: t.Optional[PlanBuilder] = None
         self._prod_plan_with_gaps_builder: t.Optional[PlanBuilder] = None
         self._check_run_mapping: t.Dict[str, CheckRun] = {}
+        self._linter_errors = None
 
         if not isinstance(get_console(), MarkdownConsole):
             raise CICDBotError("Console must be a markdown console.")
@@ -326,10 +327,13 @@ def __init__(
             if review.state.lower() == "approved"
         }
         logger.debug(f"Approvers: {', '.join(self._approvers)}")
-        self._context: Context = Context(
-            paths=self._paths,
-            config=self.config,
-        )
+
+        self._context: Context = Context(paths=self._paths, config=self.config, load=False)
+        self._console.clear_captured_outputs()
+        try:
+            self._context.load()
+        except LinterError:
+            self._linter_errors = self._console.captured_errors
 
     @property
     def deploy_command_enabled(self) -> bool:
@@ -654,6 +658,40 @@ def _update_check_handler(
             full_summary=summary,
         )
 
+    def update_linter_check(
+        self,
+        status: GithubCheckStatus,
+        conclusion: t.Optional[GithubCheckConclusion] = None,
+    ) -> None:
+        if not self._context.config.linter.enabled:
+            # Linter is not enabled, skip check
+            return
+
+        def conclusion_handler(
+            conclusion: GithubCheckConclusion,
+        ) -> t.Tuple[GithubCheckConclusion, str, t.Optional[str]]:
+            test_summary = (
+                self._linter_errors or self._console.consume_captured_output() or "Linter Success"
+            )
+
+            title = "Linter results"
+
+            return conclusion, title, test_summary
+
+        self._update_check_handler(
+            check_name="SQLMesh - Linter",
+            status=status,
+            conclusion=conclusion,
+            status_handler=lambda status: (
+                {
+                    GithubCheckStatus.IN_PROGRESS: "Running linter",
+                    GithubCheckStatus.QUEUED: "Waiting to Run linter",
+                }[status],
+                None,
+            ),
+            conclusion_handler=conclusion_handler,
+        )
+
     def update_test_check(
         self,
         status: GithubCheckStatus,
@@ -751,7 +789,7 @@ def update_pr_environment_check(
         Updates the status of the merge commit for the PR environment.
         """
         conclusion: t.Optional[GithubCheckConclusion] = None
-        if isinstance(exception, (NoChangesPlanError, TestFailure)):
+        if isinstance(exception, (NoChangesPlanError, TestFailure, LinterError)):
             conclusion = GithubCheckConclusion.SKIPPED
         elif isinstance(exception, UncategorizedPlanError):
             conclusion = GithubCheckConclusion.ACTION_REQUIRED
diff --git a/tests/integrations/github/cicd/fixtures.py b/tests/integrations/github/cicd/fixtures.py
@@ -3,6 +3,7 @@
 import pytest
 from pytest_mock.plugin import MockerFixture
 
+from sqlmesh.core.config import Config
 from sqlmesh.core.console import set_console, get_console, MarkdownConsole
 from sqlmesh.integrations.github.cicd.config import GithubCICDBotConfig
 from sqlmesh.integrations.github.cicd.controller import (
@@ -67,6 +68,7 @@ def _make_function(
         merge_state_status: MergeStateStatus = MergeStateStatus.CLEAN,
         bot_config: t.Optional[GithubCICDBotConfig] = None,
         mock_out_context: bool = True,
+        config: t.Optional[t.Union[Config, str]] = None,
     ) -> GithubController:
         if mock_out_context:
             mocker.patch("sqlmesh.core.context.Context.apply", mocker.MagicMock())
@@ -97,6 +99,7 @@ def _make_function(
                     else GithubEvent.from_obj(event_path)
                 ),
                 client=client,
+                config=config,
             )
         finally:
             set_console(orig_console)
diff --git a/tests/integrations/github/cicd/test_integration.py b/tests/integrations/github/cicd/test_integration.py
@@ -13,7 +13,7 @@
 from pytest_mock.plugin import MockerFixture
 from sqlglot import exp
 
-from sqlmesh.core.config import CategorizerConfig
+from sqlmesh.core.config import CategorizerConfig, Config, ModelDefaultsConfig, LinterConfig
 from sqlmesh.core.engine_adapter.shared import DataObject
 from sqlmesh.core.user import User, UserRole
 from sqlmesh.integrations.github.cicd import command
@@ -52,6 +52,144 @@ def get_columns(
     return controller._context.engine_adapter.columns(table)
 
 
+@time_machine.travel("2023-01-01 15:00:00 UTC")
+def test_linter(
+    github_client,
+    make_controller,
+    make_mock_check_run,
+    make_mock_issue_comment,
+    make_pull_request_review,
+    tmp_path: pathlib.Path,
+    mocker: MockerFixture,
+):
+    """
+    PR with a non-breaking change and auto-categorization will be backfilled, merged, and deployed to prod
+
+    Scenario:
+    - PR is not merged
+    - PR has been approved by a required reviewer
+    - Tests passed
+    - PR Merge Method defined
+    - Delete environment is disabled
+    - Changes made in PR with auto-categorization
+    """
+
+    mock_repo = github_client.get_repo()
+    mock_repo.create_check_run = mocker.MagicMock(
+        side_effect=lambda **kwargs: make_mock_check_run(**kwargs)
+    )
+
+    created_comments: t.List[MockIssueComment] = []
+    mock_issue = mock_repo.get_issue()
+    mock_issue.create_comment = mocker.MagicMock(
+        side_effect=lambda comment: make_mock_issue_comment(
+            comment=comment, created_comments=created_comments
+        )
+    )
+    mock_issue.get_comments = mocker.MagicMock(side_effect=lambda: created_comments)
+
+    mock_pull_request = mock_repo.get_pull()
+    mock_pull_request.get_reviews = mocker.MagicMock(
+        side_effect=lambda: [make_pull_request_review(username="test_github", state="APPROVED")]
+    )
+    mock_pull_request.merged = False
+    mock_pull_request.merge = mocker.MagicMock()
+
+    # Case 1: Test for linter errors
+    config = Config(
+        model_defaults=ModelDefaultsConfig(dialect="duckdb"),
+        linter=LinterConfig(enabled=True, rules="ALL"),
+    )
+
+    controller = make_controller(
+        "tests/fixtures/github/pull_request_synchronized.json",
+        github_client,
+        bot_config=GithubCICDBotConfig(
+            merge_method=MergeMethod.MERGE,
+            invalidate_environment_after_deploy=False,
+            auto_categorize_changes=CategorizerConfig.all_full(),
+            default_pr_start=None,
+            skip_pr_backfill=False,
+        ),
+        mock_out_context=False,
+        config=config,
+    )
+
+    github_output_file = tmp_path / "github_output.txt"
+
+    with mock.patch.dict(os.environ, {"GITHUB_OUTPUT": str(github_output_file)}):
+        with pytest.raises(CICDBotError):
+            command._run_all(controller)
+
+    assert "SQLMesh - Linter" in controller._check_run_mapping
+    linter_checks_runs = controller._check_run_mapping["SQLMesh - Linter"].all_kwargs
+    assert "Linter errors for" in linter_checks_runs[-1]["output"]["summary"]
+    assert GithubCheckConclusion(linter_checks_runs[-1]["conclusion"]).is_failure
+
+    for check in (
+        "SQLMesh - Run Unit Tests",
+        "SQLMesh - PR Environment Synced",
+        "SQLMesh - Prod Plan Preview",
+    ):
+        assert check in controller._check_run_mapping
+        check_runs = controller._check_run_mapping[check].all_kwargs
+        assert GithubCheckConclusion(check_runs[-1]["conclusion"]).is_skipped
+
+    with open(github_output_file, "r", encoding="utf-8") as f:
+        output = f.read()
+        assert (
+            output
+            == "linter=failure\npr_environment_name=hello_world_2\npr_environment_synced=skipped\nprod_plan_preview=skipped\nrun_unit_tests=skipped\n"
+        )
+
+    # empty github file for next case
+    open(github_output_file, "w").close()
+
+    # Case 2: Test for linter warnings
+    config = Config(
+        model_defaults=ModelDefaultsConfig(dialect="duckdb"),
+        linter=LinterConfig(enabled=True, warn_rules="ALL"),
+    )
+
+    controller = make_controller(
+        "tests/fixtures/github/pull_request_synchronized.json",
+        github_client,
+        bot_config=GithubCICDBotConfig(
+            merge_method=MergeMethod.MERGE,
+            invalidate_environment_after_deploy=False,
+            auto_categorize_changes=CategorizerConfig.all_full(),
+            default_pr_start=None,
+            skip_pr_backfill=False,
+        ),
+        mock_out_context=False,
+        config=config,
+    )
+
+    with mock.patch.dict(os.environ, {"GITHUB_OUTPUT": str(github_output_file)}):
+        command._run_all(controller)
+
+    assert "SQLMesh - Linter" in controller._check_run_mapping
+    linter_checks_runs = controller._check_run_mapping["SQLMesh - Linter"].all_kwargs
+    assert "Linter warnings for" in linter_checks_runs[-1]["output"]["summary"]
+    assert GithubCheckConclusion(linter_checks_runs[-1]["conclusion"]).is_success
+
+    for check in (
+        "SQLMesh - Run Unit Tests",
+        "SQLMesh - PR Environment Synced",
+        "SQLMesh - Prod Plan Preview",
+    ):
+        assert check in controller._check_run_mapping
+        check_runs = controller._check_run_mapping[check].all_kwargs
+        assert GithubCheckConclusion(check_runs[-1]["conclusion"]).is_success
+
+    with open(github_output_file, "r", encoding="utf-8") as f:
+        output = f.read()
+        assert (
+            output
+            == "linter=success\nrun_unit_tests=success\ncreated_pr_environment=true\npr_environment_name=hello_world_2\npr_environment_synced=success\nprod_plan_preview=success\n"
+        )
+
+
 @time_machine.travel("2023-01-01 15:00:00 UTC")
 def test_merge_pr_has_non_breaking_change(
     github_client,
