Feat: Add CICD bot linter logging

VaggelisD · VaggelisD · commit ba0b56de34a7 · 2025-03-28T17:19:36.000-04:00
diff --git a/sqlmesh/core/console.py b/sqlmesh/core/console.py
@@ -2164,7 +2164,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"
diff --git a/sqlmesh/core/context.py b/sqlmesh/core/context.py
@@ -2394,7 +2394,10 @@ def _get_models_for_interval_end(
             )
         return models_for_interval_end
 
-    def lint_models(self, models: t.Optional[t.Iterable[t.Union[str, Model]]] = None) -> None:
+    def lint_models(
+        self,
+        models: t.Optional[t.Iterable[t.Union[str, Model]]] = None,
+    ) -> None:
         found_error = False
 
         model_list = (
@@ -2403,7 +2406,7 @@ def lint_models(self, models: t.Optional[t.Iterable[t.Union[str, Model]]] = None
         for model in model_list:
             # Linter may be `None` if the context is not loaded yet
             if linter := self._linters.get(model.project):
-                found_error = linter.lint_model(model) or found_error
+                found_error = linter.lint_model(model, console=self.console) or found_error
 
         if found_error:
             raise LinterError(
diff --git a/sqlmesh/core/linter/definition.py b/sqlmesh/core/linter/definition.py
@@ -7,7 +7,7 @@
 from sqlmesh.core.model import Model
 
 from sqlmesh.utils.errors import raise_config_error
-from sqlmesh.core.console import get_console
+from sqlmesh.core.console import get_console, Console
 from sqlmesh.core.linter.rule import RuleSet
 
 
@@ -50,7 +50,7 @@ def from_rules(cls, all_rules: RuleSet, config: LinterConfig) -> Linter:
 
         return Linter(config.enabled, all_rules, rules, warn_rules)
 
-    def lint_model(self, model: Model) -> bool:
+    def lint_model(self, model: Model, console: Console = get_console()) -> bool:
         if not self.enabled:
             return False
 
@@ -63,10 +63,10 @@ def lint_model(self, model: Model) -> bool:
         warn_violations = warn_rules.check_model(model)
 
         if warn_violations:
-            get_console().show_linter_violations(warn_violations, model)
+            console.show_linter_violations(warn_violations, model)
 
         if error_violations:
-            get_console().show_linter_violations(error_violations, model, is_error=True)
+            console.show_linter_violations(error_violations, model, is_error=True)
             return True
 
         return False
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__)
 
@@ -80,6 +80,25 @@ def _run_tests(controller: GithubController) -> bool:
         return False
 
 
+def _run_linter(controller: GithubController) -> bool:
+    controller.update_linter_check(status=GithubCheckStatus.IN_PROGRESS)
+    try:
+        controller.run_linter()
+    except LinterError:
+        controller.update_linter_check(
+            status=GithubCheckStatus.COMPLETED,
+            conclusion=GithubCheckConclusion.FAILURE,
+        )
+        return False
+
+    controller.update_linter_check(
+        status=GithubCheckStatus.COMPLETED,
+        conclusion=GithubCheckConclusion.SUCCESS,
+    )
+
+    return True
+
+
 @github.command()
 @click.pass_context
 @cli_analytics
@@ -204,12 +223,15 @@ def _run_all(controller: GithubController) -> None:
             has_required_approval = True
         else:
             raise CICDBotError(f"Unsupported command: {command}")
+    controller.update_linter_check(status=GithubCheckStatus.QUEUED)
     controller.update_pr_environment_check(status=GithubCheckStatus.QUEUED)
     controller.update_prod_plan_preview_check(status=GithubCheckStatus.QUEUED)
     controller.update_test_check(status=GithubCheckStatus.QUEUED)
     if is_auto_deploying_prod:
         controller.update_prod_environment_check(status=GithubCheckStatus.QUEUED)
+    linter_passed = _run_linter(controller)
     tests_passed = _run_tests(controller)
+    print(f"linter_passed {linter_passed} tests_passed {tests_passed}")
     if controller.do_required_approval_check:
         if has_required_approval:
             controller.update_required_approval_check(
@@ -218,23 +240,25 @@ def _run_all(controller: GithubController) -> None:
         else:
             controller.update_required_approval_check(status=GithubCheckStatus.QUEUED)
             has_required_approval = _check_required_approvers(controller)
-    if not tests_passed:
+    if not tests_passed or not linter_passed:
         controller.update_pr_environment_check(
             status=GithubCheckStatus.COMPLETED,
-            exception=TestFailure(),
+            exception=LinterError("") if not linter_passed 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="Linter or unit test(s) 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="Linter or unit test(s) Failed so skipping deploying to production",
             )
-        raise CICDBotError("Failed to run tests. See check status for more information.")
+
+        raise CICDBotError("Linter or unit test(s) failed. See check status for more information.")
+
     pr_environment_updated = _update_pr_environment(controller)
     prod_plan_generated = False
     if pr_environment_updated:
@@ -271,6 +295,9 @@ def _run_all(controller: GithubController) -> None:
         or not prod_plan_generated
         or (has_required_approval and not deployed_to_prod)
     ):
+        print(
+            f"not pr_environment_updated {not pr_environment_updated} prod_plan_generated {not prod_plan_generated} has_required_approval {has_required_approval} deployed_to_prod {not deployed_to_prod}"
+        )
         raise CICDBotError(
             "A step of the run-all check failed. See check status for more information."
         )
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, Verbosity
 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__)
 
 
@@ -326,10 +326,7 @@ 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)
 
     @property
     def deploy_command_enabled(self) -> bool:
@@ -394,6 +391,7 @@ def pr_plan(self) -> Plan:
             self._pr_plan_builder = self._context.plan_builder(
                 environment=self.pr_environment_name,
                 skip_tests=True,
+                skip_linter=True,
                 categorizer_config=self.bot_config.auto_categorize_changes,
                 start=self.bot_config.default_pr_start,
                 skip_backfill=self.bot_config.skip_pr_backfill,
@@ -409,6 +407,7 @@ def prod_plan(self) -> Plan:
                 c.PROD,
                 no_gaps=True,
                 skip_tests=True,
+                skip_linter=True,
                 categorizer_config=self.bot_config.auto_categorize_changes,
                 run=self.bot_config.run_on_deploy_to_prod,
             )
@@ -423,6 +422,7 @@ def prod_plan_with_gaps(self) -> Plan:
                 no_gaps=False,
                 no_auto_categorization=True,
                 skip_tests=True,
+                skip_linter=True,
                 run=self.bot_config.run_on_deploy_to_prod,
             )
         assert self._prod_plan_with_gaps_builder
@@ -478,6 +478,13 @@ def run_tests(self) -> t.Tuple[unittest.result.TestResult, str]:
         """
         return self._context._run_tests(verbosity=Verbosity.VERBOSE)
 
+    def run_linter(self) -> None:
+        """
+        Run linter for the PR
+        """
+        self._console.clear_captured_outputs()
+        self._context.lint_models()
+
     def _get_or_create_comment(self, header: str = BOT_HEADER_MSG) -> IssueComment:
         comment = seq_get(
             [comment for comment in self._issue.get_comments() if header in comment.body],
@@ -654,6 +661,37 @@ 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:
+            return
+
+        def conclusion_handler(
+            conclusion: GithubCheckConclusion,
+        ) -> t.Tuple[GithubCheckConclusion, str, t.Optional[str]]:
+            linter_summary = self._console.consume_captured_output() or "Linter Success"
+
+            title = "Linter results"
+
+            return conclusion, title, linter_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
