Chore: Refine environment statements cli output (#4153)

Author: themisvaltinos

sqlmesh/core/console.py

@@ -281,6 +281,14 @@ def update_env_migration_progress(self, num_tasks: int) -> None:
     def stop_env_migration_progress(self, success: bool = True) -> None:
         """Stop the environment migration progress."""
 
+    @abc.abstractmethod
+    def show_environment_difference_summary(
+        self,
+        context_diff: ContextDiff,
+        no_diff: bool = True,
+    ) -> None:
+        """Displays a summary of differences for the environment."""
+
     @abc.abstractmethod
     def show_model_difference_summary(
         self,
@@ -551,13 +559,19 @@ def update_state_import_progress(
     def stop_state_import(self, success: bool, input_file: Path) -> None:
         pass
 
+    def show_environment_difference_summary(
+        self,
+        context_diff: ContextDiff,
+        no_diff: bool = True,
+    ) -> None:
+        pass
+
     def show_model_difference_summary(
         self,
         context_diff: ContextDiff,
         environment_naming_info: EnvironmentNamingInfo,
         default_catalog: t.Optional[str],
         no_diff: bool = True,
-        ignored_snapshot_ids: t.Optional[t.Set[SnapshotId]] = None,
     ) -> None:
         pass
 
@@ -1324,20 +1338,16 @@ def stop_state_import(self, success: bool, input_file: Path) -> None:
             else:
                 self.log_error("State import failed!")
 
-    def show_model_difference_summary(
+    def show_environment_difference_summary(
         self,
         context_diff: ContextDiff,
-        environment_naming_info: EnvironmentNamingInfo,
-        default_catalog: t.Optional[str],
         no_diff: bool = True,
     ) -> None:
-        """Shows a summary of the differences.
+        """Shows a summary of the environment differences.
 
         Args:
             context_diff: The context diff to use to print the summary
-            environment_naming_info: The environment naming info to reference when printing model names
-            default_catalog: The default catalog to reference when deciding to remove catalog from display names
-            no_diff: Hide the actual SQL differences.
+            no_diff: Hide the actual environment statement differences.
         """
         if context_diff.is_new_environment:
             msg = (
@@ -1372,11 +1382,28 @@ def show_model_difference_summary(
         if context_diff.has_requirement_changes:
             self._print(f"[bold]Requirements:\n{context_diff.requirements_diff()}")
 
-        if context_diff.has_environment_statements_changes:
+        if context_diff.has_environment_statements_changes and not no_diff:
             self._print("[bold]Environment statements:\n")
-            for type, diff in context_diff.environment_statements_diff():
+            for type, diff in context_diff.environment_statements_diff(
+                include_python_env=not context_diff.is_new_environment
+            ):
                 self._print(Syntax(diff, type, line_numbers=False))
 
+    def show_model_difference_summary(
+        self,
+        context_diff: ContextDiff,
+        environment_naming_info: EnvironmentNamingInfo,
+        default_catalog: t.Optional[str],
+        no_diff: bool = True,
+    ) -> None:
+        """Shows a summary of the model differences.
+
+        Args:
+            context_diff: The context diff to use to print the summary
+            environment_naming_info: The environment naming info to reference when printing model names
+            default_catalog: The default catalog to reference when deciding to remove catalog from display names
+            no_diff: Hide the actual SQL differences.
+        """
         self._show_summary_tree_for(
             context_diff,
             "Models",
@@ -1552,12 +1579,18 @@ def _prompt_categorize(
         """Get the user's change category for the directly modified models."""
         plan = plan_builder.build()
 
-        self.show_model_difference_summary(
+        self.show_environment_difference_summary(
             plan.context_diff,
-            plan.environment_naming_info,
-            default_catalog=default_catalog,
+            no_diff=no_diff,
         )
 
+        if plan.context_diff.has_changes:
+            self.show_model_difference_summary(
+                plan.context_diff,
+                plan.environment_naming_info,
+                default_catalog=default_catalog,
+            )
+
         if not no_diff:
             self._show_categorized_snapshots(plan, default_catalog)
 
@@ -2497,20 +2530,16 @@ class MarkdownConsole(CaptureTerminalConsole):
     def __init__(self, **kwargs: t.Any) -> None:
         super().__init__(**{**kwargs, "console": RichConsole(no_color=True)})
 
-    def show_model_difference_summary(
+    def show_environment_difference_summary(
         self,
         context_diff: ContextDiff,
-        environment_naming_info: EnvironmentNamingInfo,
-        default_catalog: t.Optional[str],
         no_diff: bool = True,
     ) -> None:
-        """Shows a summary of the differences.
+        """Shows a summary of the environment differences.
 
         Args:
             context_diff: The context diff to use to print the summary.
-            environment_naming_info: The environment naming info to reference when printing model names
-            default_catalog: The default catalog to reference when deciding to remove catalog from display names
-            no_diff: Hide the actual SQL differences.
+            no_diff: Hide the actual environment statements differences.
         """
         if context_diff.is_new_environment:
             self._print(
@@ -2528,11 +2557,28 @@ def show_model_difference_summary(
         if context_diff.has_requirement_changes:
             self._print(f"Requirements:\n{context_diff.requirements_diff()}")
 
-        if context_diff.has_environment_statements_changes:
+        if context_diff.has_environment_statements_changes and not no_diff:
             self._print("[bold]Environment statements:\n")
-            for _, diff in context_diff.environment_statements_diff():
+            for _, diff in context_diff.environment_statements_diff(
+                include_python_env=not context_diff.is_new_environment
+            ):
                 self._print(diff)
 
+    def show_model_difference_summary(
+        self,
+        context_diff: ContextDiff,
+        environment_naming_info: EnvironmentNamingInfo,
+        default_catalog: t.Optional[str],
+        no_diff: bool = True,
+    ) -> None:
+        """Shows a summary of the model differences.
+
+        Args:
+            context_diff: The context diff to use to print the summary.
+            environment_naming_info: The environment naming info to reference when printing model names
+            default_catalog: The default catalog to reference when deciding to remove catalog from display names
+            no_diff: Hide the actual SQL differences.
+        """
         added_snapshots = {context_diff.snapshots[s_id] for s_id in context_diff.added}
         added_snapshot_models = {s for s in added_snapshots if s.is_model}
         if added_snapshot_models:
@@ -3035,23 +3081,32 @@ def update_env_migration_progress(self, num_tasks: int) -> None:
     def stop_env_migration_progress(self, success: bool = True) -> None:
         self._write(f"Stopping environment migration with success={success}")
 
-    def show_model_difference_summary(
+    def show_environment_difference_summary(
         self,
         context_diff: ContextDiff,
-        environment_naming_info: EnvironmentNamingInfo,
-        default_catalog: t.Optional[str],
         no_diff: bool = True,
     ) -> None:
-        self._write("Model Difference Summary:")
+        self._write("Environment Difference Summary:")
 
         if context_diff.has_requirement_changes:
             self._write(f"Requirements:\n{context_diff.requirements_diff()}")
 
-        if context_diff.has_environment_statements_changes:
+        if context_diff.has_environment_statements_changes and not no_diff:
             self._write("Environment statements:\n")
-            for _, diff in context_diff.environment_statements_diff():
+            for _, diff in context_diff.environment_statements_diff(
+                include_python_env=not context_diff.is_new_environment
+            ):
                 self._write(diff)
 
+    def show_model_difference_summary(
+        self,
+        context_diff: ContextDiff,
+        environment_naming_info: EnvironmentNamingInfo,
+        default_catalog: t.Optional[str],
+        no_diff: bool = True,
+    ) -> None:
+        self._write("Model Difference Summary:")
+
         for added in context_diff.new_snapshots:
             self._write(f"  Added: {added}")
         for removed in context_diff.removed_snapshots:

sqlmesh/core/context.py

@@ -1533,17 +1533,22 @@ def diff(self, environment: t.Optional[str] = None, detailed: bool = False) -> b
         environment = environment or self.config.default_target_environment
         environment = Environment.sanitize_name(environment)
         context_diff = self._context_diff(environment)
-        self.console.show_model_difference_summary(
+        self.console.show_environment_difference_summary(
             context_diff,
-            EnvironmentNamingInfo.from_environment_catalog_mapping(
-                self.config.environment_catalog_mapping,
-                name=environment,
-                suffix_target=self.config.environment_suffix_target,
-                normalize_name=context_diff.normalize_environment_name,
-            ),
-            self.default_catalog,
             no_diff=not detailed,
         )
+        if context_diff.has_changes:
+            self.console.show_model_difference_summary(
+                context_diff,
+                EnvironmentNamingInfo.from_environment_catalog_mapping(
+                    self.config.environment_catalog_mapping,
+                    name=environment,
+                    suffix_target=self.config.environment_suffix_target,
+                    normalize_name=context_diff.normalize_environment_name,
+                ),
+                self.default_catalog,
+                no_diff=not detailed,
+            )
         return context_diff.has_changes
 
     @python_api_analytics

sqlmesh/core/context_diff.py

@@ -14,7 +14,7 @@
 
 import sys
 import typing as t
-from difflib import ndiff
+from difflib import ndiff, unified_diff
 from functools import cached_property
 from sqlmesh.core import constants as c
 from sqlmesh.core.console import get_console
@@ -341,16 +341,19 @@ def requirements_diff(self) -> str:
             )
         )
 
-    def environment_statements_diff(self) -> t.List[t.Tuple[str, str]]:
+    def environment_statements_diff(
+        self, include_python_env: bool = False
+    ) -> t.List[t.Tuple[str, str]]:
         def extract_statements(statements: t.List[EnvironmentStatements], attr: str) -> t.List[str]:
             return [
-                expr
+                string
                 for statement in statements
                 for expr in (
                     sorted_python_env_payloads(statement.python_env)
                     if attr == "python_env"
                     else getattr(statement, attr)
                 )
+                for string in expr.split("\n")
             ]
 
         def compute_diff(attribute: str) -> t.Optional[t.Tuple[str, str]]:
@@ -360,21 +363,24 @@ def compute_diff(attribute: str) -> t.Optional[t.Tuple[str, str]]:
             if previous == current:
                 return None
 
-            diff_lines = list(ndiff(previous, current))
             diff_text = attribute if not attribute == "python_env" else "dependencies"
             diff_text += ":\n"
+            if attribute == "python_env":
+                diff = list(unified_diff(previous, current))
+                diff_text += "\n".join(diff[2:] if len(diff) > 1 else diff)
+                return "python", diff_text + "\n"
 
+            diff_lines = list(ndiff(previous, current))
             if any(line.startswith(("-", "+")) for line in diff_lines):
                 diff_text += "  " + "\n  ".join(diff_lines) + "\n"
-
-            return "python" if attribute == "python_env" else "sql", diff_text
+            return "sql", diff_text
 
         return [
             diff
             for attribute in [
                 RuntimeStage.BEFORE_ALL.value,
                 RuntimeStage.AFTER_ALL.value,
-                "python_env",
+                *(["python_env"] if include_python_env else []),
             ]
             if (diff := compute_diff(attribute)) is not None
         ]

sqlmesh/integrations/github/cicd/controller.py

@@ -457,12 +457,17 @@ def get_plan_summary(self, plan: Plan) -> str:
         try:
             # Clear out any output that might exist from prior steps
             self._console.clear_captured_outputs()
-            self._console.show_model_difference_summary(
+            self._console.show_environment_difference_summary(
                 context_diff=plan.context_diff,
-                environment_naming_info=plan.environment_naming_info,
-                default_catalog=self._context.default_catalog,
                 no_diff=False,
             )
+            if plan.context_diff.has_changes:
+                self._console.show_model_difference_summary(
+                    context_diff=plan.context_diff,
+                    environment_naming_info=plan.environment_naming_info,
+                    default_catalog=self._context.default_catalog,
+                    no_diff=False,
+                )
             difference_summary = self._console.consume_captured_output()
             self._console._show_missing_dates(plan, self._context.default_catalog)
             missing_dates = self._console.consume_captured_output()

tests/core/test_context.py

@@ -287,6 +287,7 @@ def test_diff(sushi_context: Context, mocker: MockerFixture):
 
     sushi_context.upsert_model("sushi.customers", query=parse_one("select 1 as customer_id"))
     sushi_context.diff("test")
+    assert mock_console.show_environment_difference_summary.called
     assert mock_console.show_model_difference_summary.called
     assert success
 

tests/core/test_plan.py

@@ -3023,15 +3023,33 @@ def test_plan_environment_statements_diff(make_snapshot):
         terminal_console._print(diff)
     output = console_output.getvalue()
     stripped = strip_ansi_codes(output)
+
+    expected_output = (
+        "before_all:\n"
+        "  + CREATE OR REPLACE TABLE table_1 AS SELECT 1\n"
+        "  + @test_macro()\n\n"
+        "after_all:\n"
+        "  + CREATE OR REPLACE TABLE table_2 AS SELECT 2"
+    )
+    assert stripped == expected_output
+    console_output.close()
+
+    # Validate with python env included
+    console_output, terminal_console = create_test_console()
+    for _, diff in context_diff.environment_statements_diff(include_python_env=True):
+        terminal_console._print(diff)
+    output = console_output.getvalue()
+    stripped = strip_ansi_codes(output)
     expected_output = (
         "before_all:\n"
         "  + CREATE OR REPLACE TABLE table_1 AS SELECT 1\n"
         "  + @test_macro()\n\n"
         "after_all:\n"
         "  + CREATE OR REPLACE TABLE table_2 AS SELECT 2\n\n"
         "dependencies:\n"
-        "  + def test_macro(evaluator):\n"
-        "    return 'one'"
+        "@@ -0,0 +1,2 @@\n\n"
+        "+def test_macro(evaluator):\n"
+        "+    return 'one'"
     )
     assert stripped == expected_output
     console_output.close()