Chore(cicd_bot): Tidy up failure output (#4964)

Author: erindru

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:
@@ -3090,15 +3096,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:
@@ -3135,6 +3149,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))}
         )
@@ -3409,6 +3428,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)
 
@@ -3435,19 +3460,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

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,

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

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,

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