Feat: Add CICD bot check for linting (#4027)

Author: VaggelisD

docs/integrations/github.md

@@ -5,6 +5,7 @@
 The GitHub Actions CI/CD Bot enables teams to automate their SQLMesh projects using GitHub Actions. It can be configured to perform the following things:
 
 * Automatically run unit tests on PRs
+* Automatically run the linter on PRs
 * Automatically create PR environments that represent the code changes in the PR
 * Automatically categorize and backfill data for models that have changed
 * Automatically deploy changes to production with automatic data gap prevention and merge the PR
@@ -350,6 +351,7 @@ These can be used to potentially trigger follow up steps in the workflow.
 These are the possible outputs (based on how the bot is configured) that are created by the bot:
 
 * `run_unit_tests`
+* `linter`
 * `has_required_approval`
 * `pr_environment_synced`
 * `prod_plan_preview`
@@ -373,6 +375,8 @@ In addition, there are custom outputs listed below:
 * `created_pr_environment` - set to `"true"` (a string with a value of `true`) if a PR environment was created for the first time. It is absent, or considered empty string if you check for it, if it is not created for the first time
 * `pr_environment_name` - the name of the PR environment. It is output whenever PR environment synced check reaches a conclusion. Therefore make sure to check the status of `created_pr_environment` or `pr_environment_synced` before acting on this output 
 
+Note: The `linter` step will run only if it's enabled in the project's configuration (`config.yaml` / `config.py`). The step will fail if the linter finds errors, otherwise it'll output only the warnings.
+
 ## Custom Workflow Configuration
 You can configure each individual action to run as a separate step. This can allow for more complex workflows or integrating specific steps with other actions you want to trigger. Run `sqlmesh_cicd github` to see a list of commands that can be supplied and their potential options.
 ```bash
@@ -460,6 +464,10 @@ jobs:
 ## Example Screenshots
 ### Automated Unit Tests with Error Summary
 ![Automated Unit Tests with Error Summary](github/github_test_summary.png)
+### Automated Linting with Error Summary
+![Automated Linting with Error Summary](github/linter_errors.png)
+### Automated Linting with Warning Summary
+![Automated Linting with Warning Summary](github/linter_warnings.png)
 ### Automatically create PR Environments that represent the code changes in the PR
 ![Environment Summary](github/github_env_summary.png)
 ### Enforce that certain reviewers have approved of the PR before it can be merged

docs/integrations/github/linter_errors.png

@@ -2241,7 +2241,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"

docs/integrations/github/linter_warnings.png

@@ -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(

sqlmesh/core/console.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

sqlmesh/core/context.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,11 +223,13 @@ 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)
     if controller.do_required_approval_check:
         if has_required_approval:
@@ -218,23 +239,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:

sqlmesh/core/linter/definition.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

sqlmesh/integrations/github/cicd/command.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)

sqlmesh/integrations/github/cicd/controller.py

@@ -497,7 +497,7 @@ def test_run_all_test_failed(
     )
     assert (
         prod_plan_preview_checks_runs[1]["output"]["summary"]
-        == "Unit Test(s) Failed so skipping creating prod plan"
+        == "Linter or Unit Test(s) failed so skipping creating prod plan"
     )
 
     assert "SQLMesh - PR Environment Synced" in controller._check_run_mapping
@@ -625,7 +625,7 @@ def test_run_all_test_exception(
     )
     assert (
         prod_plan_preview_checks_runs[1]["output"]["summary"]
-        == "Unit Test(s) Failed so skipping creating prod plan"
+        == "Linter or Unit Test(s) failed so skipping creating prod plan"
     )
 
     assert "SQLMesh - PR Environment Synced" in controller._check_run_mapping