From e9b23813c8f0a328e28e2bfba2147249dcfa5397 Mon Sep 17 00:00:00 2001 From: Erin Drummond Date: Mon, 14 Jul 2025 03:31:55 +0000 Subject: [PATCH] Chore(cicd_bot): Tidy up failure output --- sqlmesh/core/console.py | 52 +++++++++++++++---- sqlmesh/integrations/github/cicd/command.py | 2 +- .../integrations/github/cicd/controller.py | 5 +- tests/integrations/github/cicd/conftest.py | 2 +- .../github/cicd/test_integration.py | 3 +- 5 files changed, 48 insertions(+), 16 deletions(-) diff --git a/sqlmesh/core/console.py b/sqlmesh/core/console.py index 86de61c829..e09ac0c3ea 100644 --- a/sqlmesh/core/console.py +++ b/sqlmesh/core/console.py @@ -289,11 +289,17 @@ def show_row_diff( class BaseConsole(abc.ABC): @abc.abstractmethod - def log_error(self, message: str) -> None: + def log_error(self, message: str, *args: t.Any, **kwargs: t.Any) -> None: """Display error info to the user.""" @abc.abstractmethod - def log_warning(self, short_message: str, long_message: t.Optional[str] = None) -> None: + def log_warning( + self, + short_message: str, + long_message: t.Optional[str] = None, + *args: t.Any, + **kwargs: t.Any, + ) -> None: """Display warning info to the user. Args: @@ -3082,15 +3088,23 @@ def consume_captured_errors(self) -> str: finally: self._errors = [] - def log_warning(self, short_message: str, long_message: t.Optional[str] = None) -> None: + def log_warning( + self, + short_message: str, + long_message: t.Optional[str] = None, + *args: t.Any, + **kwargs: t.Any, + ) -> None: if short_message not in self._warnings: self._warnings.append(short_message) - super().log_warning(short_message, long_message) + if kwargs.pop("print", True): + super().log_warning(short_message, long_message) - def log_error(self, message: str) -> None: + def log_error(self, message: str, *args: t.Any, **kwargs: t.Any) -> None: if message not in self._errors: self._errors.append(message) - super().log_error(message) + if kwargs.pop("print", True): + super().log_error(message) def log_skipped_models(self, snapshot_names: t.Set[str]) -> None: if snapshot_names: @@ -3127,6 +3141,11 @@ def __init__(self, **kwargs: t.Any) -> None: kwargs.pop("alert_block_collapsible_threshold", 200) ) + # capture_only = True: capture but dont print to console + # capture_only = False: capture and also print to console + self.warning_capture_only = kwargs.pop("warning_capture_only", False) + self.error_capture_only = kwargs.pop("error_capture_only", False) + super().__init__( **{**kwargs, "console": RichConsole(no_color=True, width=kwargs.pop("width", None))} ) @@ -3401,6 +3420,12 @@ def stop_promotion_progress(self, success: bool = True) -> None: super().stop_promotion_progress(success) self._print("\n") + def log_warning(self, short_message: str, long_message: t.Optional[str] = None) -> None: + super().log_warning(short_message, long_message, print=not self.warning_capture_only) + + def log_error(self, message: str) -> None: + super().log_error(message, print=not self.error_capture_only) + def log_success(self, message: str) -> None: self._print(message) @@ -3427,19 +3452,24 @@ def log_test_results(self, result: ModelTextTestResult, target_dialect: str) -> def log_skipped_models(self, snapshot_names: t.Set[str]) -> None: if snapshot_names: - msg = " " + "\n ".join(snapshot_names) - self._print(f"**Skipped models**\n\n{msg}") + self._print(f"**Skipped models**") + for snapshot_name in snapshot_names: + self._print(f"* `{snapshot_name}`") + self._print("") def log_failed_models(self, errors: t.List[NodeExecutionFailedError]) -> None: if errors: - self._print("\n```\nFailed models\n") + self._print("**Failed models**") error_messages = _format_node_errors(errors) for node_name, msg in error_messages.items(): - self._print(f" **{node_name}**\n\n{msg}") + self._print(f"* `{node_name}`\n") + self._print(" ```") + self._print(msg) + self._print(" ```") - self._print("```\n") + self._print("") def show_linter_violations( self, violations: t.List[RuleViolation], model: Model, is_error: bool = False diff --git a/sqlmesh/integrations/github/cicd/command.py b/sqlmesh/integrations/github/cicd/command.py index 1a4982e9e1..f1b611150a 100644 --- a/sqlmesh/integrations/github/cicd/command.py +++ b/sqlmesh/integrations/github/cicd/command.py @@ -30,7 +30,7 @@ def github(ctx: click.Context, token: str) -> None: """Github Action CI/CD Bot. See https://sqlmesh.readthedocs.io/en/stable/integrations/github/ for details""" # set a larger width because if none is specified, it auto-detects 80 characters when running in GitHub Actions # which can result in surprise newlines when outputting dates to backfill - set_console(MarkdownConsole(width=1000)) + set_console(MarkdownConsole(width=1000, warning_capture_only=True, error_capture_only=True)) ctx.obj["github"] = GithubController( paths=ctx.obj["paths"], token=token, diff --git a/sqlmesh/integrations/github/cicd/controller.py b/sqlmesh/integrations/github/cicd/controller.py index 7b03868243..56a3c1ab20 100644 --- a/sqlmesh/integrations/github/cicd/controller.py +++ b/sqlmesh/integrations/github/cicd/controller.py @@ -603,11 +603,11 @@ def _get_pr_environment_summary_action_required( def _get_pr_environment_summary_failure(self, exception: t.Optional[Exception] = None) -> str: console_output = self._console.consume_captured_output() + failure_msg = "" if isinstance(exception, PlanError): - failure_msg = f"Plan application failed.\n" if exception.args and (msg := exception.args[0]) and isinstance(msg, str): - failure_msg += f"\n{msg}\n" + failure_msg += f"*{msg}*\n" if console_output: failure_msg += f"\n{console_output}" elif isinstance(exception, (SQLMeshError, SqlglotError, ValueError)): @@ -713,6 +713,7 @@ def update_pr_environment(self) -> None: Creates a PR environment from the logic present in the PR. If the PR contains changes that are uncategorized, then an error will be raised. """ + self._console.consume_captured_output() # clear output buffer self._context.apply(self.pr_plan) # will raise if PR environment creation fails # update PR info comment diff --git a/tests/integrations/github/cicd/conftest.py b/tests/integrations/github/cicd/conftest.py index 25ba3b2d60..f869dc41ad 100644 --- a/tests/integrations/github/cicd/conftest.py +++ b/tests/integrations/github/cicd/conftest.py @@ -117,7 +117,7 @@ def _make_function( orig_console = get_console() try: - set_console(MarkdownConsole()) + set_console(MarkdownConsole(warning_capture_only=True, error_capture_only=True)) return GithubController( paths=paths, diff --git a/tests/integrations/github/cicd/test_integration.py b/tests/integrations/github/cicd/test_integration.py index 17e495fbc3..15e8be0f6b 100644 --- a/tests/integrations/github/cicd/test_integration.py +++ b/tests/integrations/github/cicd/test_integration.py @@ -1582,7 +1582,8 @@ def test_error_msg_when_applying_plan_with_bug( assert GithubCheckConclusion(pr_checks_runs[2]["conclusion"]).is_failure assert pr_checks_runs[2]["output"]["title"] == "PR Virtual Data Environment: hello_world_2" summary = pr_checks_runs[2]["output"]["summary"].replace("\n", "") - assert 'Failed models **"memory"."sushi"."waiter_revenue_by_day"**' in summary + assert '**Skipped models*** `"memory"."sushi"."top_waiters"`' in summary + assert '**Failed models*** `"memory"."sushi"."waiter_revenue_by_day"`' in summary assert 'Binder Error: Referenced column "non_existing_col" not found in FROM clause!' in summary assert "SQLMesh - Prod Plan Preview" in controller._check_run_mapping