Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Add CICD bot check for linting #4027

Merged
merged 2 commits into from
Mar 31, 2025
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Feat: Add CICD bot linter logging
  • Loading branch information
VaggelisD committed Mar 28, 2025
commit 2cf6f5106b98787ee1aeb1dc1b2a8e53b2368958
1 change: 0 additions & 1 deletion sqlmesh/core/console.py
Original file line number Diff line number Diff line change
@@ -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"
7 changes: 5 additions & 2 deletions sqlmesh/core/context.py
Original file line number Diff line number Diff line change
@@ -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(
8 changes: 4 additions & 4 deletions sqlmesh/core/linter/definition.py
Original file line number Diff line number Diff line change
@@ -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
35 changes: 29 additions & 6 deletions sqlmesh/integrations/github/cicd/command.py
Original file line number Diff line number Diff line change
@@ -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:
52 changes: 45 additions & 7 deletions sqlmesh/integrations/github/cicd/controller.py
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions tests/integrations/github/cicd/fixtures.py
Original file line number Diff line number Diff line change
@@ -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)
4 changes: 2 additions & 2 deletions tests/integrations/github/cicd/test_github_commands.py
Original file line number Diff line number Diff line change
@@ -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
Loading