Skip to content

Commit ce289dc

Browse files
committed
fix(console): preserve literal SQL in GitHub plan diffs
Signed-off-by: Vamsi-klu <nrvamsi13@gmail.com>
1 parent c0026dc commit ce289dc

3 files changed

Lines changed: 298 additions & 8 deletions

File tree

‎sqlmesh/core/console.py‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import unittest
77
import uuid
88
import logging
9+
import re
910
import textwrap
1011
from humanize import metric, naturalsize
1112
from itertools import zip_longest
@@ -3354,10 +3355,13 @@ def show_environment_difference_summary(
33543355

33553356
if context_diff.has_environment_statements_changes and not no_diff:
33563357
self._print("\nEnvironment statements:\n")
3357-
for _, diff in context_diff.environment_statements_diff(
3358+
for language, diff in context_diff.environment_statements_diff(
33583359
include_python_env=not context_diff.is_new_environment
33593360
):
3360-
self._print(diff)
3361+
if language == "sql":
3362+
self._print_diff(diff)
3363+
else:
3364+
self._print(diff)
33613365

33623366
def show_model_difference_summary(
33633367
self,
@@ -3474,12 +3478,8 @@ def _print_modified_models(
34743478
diff_text = context_diff.text_diff(snapshot.name)
34753479
# sometimes there is no text_diff, like on a seed model where the data has been updated
34763480
if diff_text:
3477-
diff_text = f"\n```diff\n{diff_text}\n```"
34783481
# these are part of a Markdown list, so indent them by 2 spaces to relate them to the current list item
3479-
diff_text_indented = "\n".join(
3480-
[f" {line}" for line in diff_text.splitlines()]
3481-
)
3482-
self._print(diff_text_indented)
3482+
self._print_diff(diff_text, indent=" ")
34833483
else:
34843484
if indirectly_modified_children:
34853485
self._print("\n")
@@ -3573,7 +3573,7 @@ def _show_categorized_snapshots(self, plan: Plan, default_catalog: t.Optional[st
35733573
else:
35743574
continue
35753575

3576-
self._print(f"```diff\n{context_diff.text_diff(snapshot.name)}\n```\n")
3576+
self._print_diff(context_diff.text_diff(snapshot.name))
35773577
self._print("```\n")
35783578
self._print(tree)
35793579
self._print("\n```")
@@ -3697,6 +3697,15 @@ def _print(self, value: t.Any, **kwargs: t.Any) -> None:
36973697
self.console.print(value, **kwargs)
36983698
self._captured_outputs.append(capture.get())
36993699

3700+
def _print_diff(self, diff: str, indent: str = "") -> None:
3701+
longest_backticks = max((len(run) for run in re.findall(r"`+", diff)), default=0)
3702+
fence = "`" * max(3, longest_backticks + 1)
3703+
indented_diff = diff.replace("\n", f"\n{indent}")
3704+
block = f"{indent}\n{indent}{fence}diff\n{indent}{indented_diff}\n{indent}{fence}\n"
3705+
# Capture the Markdown source, since even literal Rich output expands tabs.
3706+
self._captured_outputs.append(block)
3707+
self.console.print(block, markup=False, emoji=False, soft_wrap=True, end="")
3708+
37003709

37013710
class DatabricksMagicConsole(CaptureTerminalConsole):
37023711
"""

‎tests/core/test_console.py‎

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,229 @@
1+
import pytest
2+
13
from sqlmesh.core.console import MarkdownConsole
4+
from sqlmesh.core.context_diff import ContextDiff
5+
from sqlmesh.core.dialect import parse
6+
from sqlmesh.core.environment import EnvironmentNamingInfo, EnvironmentStatements
7+
from sqlmesh.core.model import load_sql_based_model
8+
from sqlmesh.core.snapshot import SnapshotChangeCategory
9+
from sqlmesh.utils.metaprogramming import Executable
10+
11+
12+
@pytest.fixture
13+
def markdown_context_diff(make_snapshot):
14+
def make_diff(old_query, new_query, dialect="tsql"):
15+
old, new = [
16+
make_snapshot(
17+
load_sql_based_model(parse(f"MODEL (name test.model, dialect {dialect}); {query}"))
18+
)
19+
for query in (old_query, new_query)
20+
]
21+
new.categorize_as(SnapshotChangeCategory.NON_BREAKING)
22+
return ContextDiff(
23+
environment="prod",
24+
is_new_environment=False,
25+
is_unfinalized_environment=False,
26+
normalize_environment_name=True,
27+
create_from="prod",
28+
create_from_env_exists=True,
29+
added=set(),
30+
removed_snapshots={},
31+
modified_snapshots={new.name: (new, old)},
32+
snapshots={new.snapshot_id: new},
33+
new_snapshots={new.snapshot_id: new},
34+
previous_plan_id=None,
35+
previously_promoted_snapshot_ids=set(),
36+
previous_finalized_snapshots=None,
37+
previous_gateway_managed_virtual_layer=False,
38+
gateway_managed_virtual_layer=False,
39+
environment_statements=[],
40+
)
41+
42+
return make_diff
43+
44+
45+
@pytest.mark.parametrize(
46+
"dialect,old_column,new_column,table",
47+
[
48+
("tsql", "[old_col]", "[new_col]", "[schema].[table]"),
49+
("tsql", "[old column]", "[new]]column]", "[my schema].[my]]table]"),
50+
("duckdb", "old_col", "new_col", "schema_name.table_name"),
51+
],
52+
)
53+
def test_markdown_console_model_diff_identifiers(
54+
markdown_context_diff, dialect, old_column, new_column, table
55+
):
56+
context_diff = markdown_context_diff(
57+
f"SELECT {old_column} FROM {table}",
58+
f"SELECT {new_column} FROM {table}",
59+
dialect,
60+
)
61+
snapshot = next(iter(context_diff.snapshots.values()))
62+
diff = context_diff.text_diff(snapshot.name)
63+
assert old_column in diff and new_column in diff and table in diff
64+
65+
console = MarkdownConsole()
66+
console.show_model_difference_summary(
67+
context_diff, EnvironmentNamingInfo(), None, no_diff=False
68+
)
69+
70+
output = console.consume_captured_output()
71+
assert (
72+
" ```diff\n" + "\n".join(f" {line}" for line in diff.split("\n")) + "\n ```\n" in output
73+
)
74+
assert "**Directly Modified:**" in output
75+
assert "(Non-breaking)" in output
76+
assert console.consume_captured_output() == ""
77+
78+
79+
@pytest.mark.parametrize("categorized", [False, True])
80+
@pytest.mark.parametrize("backticks", ["", "```", "`````"])
81+
def test_markdown_console_literal_model_diffs(
82+
markdown_context_diff, mocker, capsys, categorized, backticks
83+
):
84+
literal = f"[bold] [/invalid] :smile: \\path\t{'long text ' * 20}\n{backticks}\nend"
85+
context_diff = markdown_context_diff("SELECT 1 AS [old]", f"SELECT '{literal}' AS [new]")
86+
snapshot = next(iter(context_diff.snapshots.values()))
87+
diff = context_diff.text_diff(snapshot.name)
88+
assert all(line in diff for line in literal.split("\n"))
89+
console = MarkdownConsole(width=30)
90+
console.log_success("before diff")
91+
if categorized:
92+
plan = mocker.Mock(
93+
context_diff=context_diff,
94+
categorized=[snapshot],
95+
indirectly_modified={},
96+
environment_naming_info=EnvironmentNamingInfo(),
97+
)
98+
console._show_categorized_snapshots(plan, None)
99+
else:
100+
console.show_model_difference_summary(
101+
context_diff, EnvironmentNamingInfo(), None, no_diff=False
102+
)
103+
console.log_success("after diff")
104+
105+
output = console.consume_captured_output()
106+
fence = "`" * max(3, len(backticks) + 1)
107+
indent = "" if categorized else " "
108+
block = (
109+
f"{indent}{fence}diff\n"
110+
+ "\n".join(f"{indent}{line}" for line in diff.split("\n"))
111+
+ f"\n{indent}{fence}\n"
112+
)
113+
assert block in output
114+
assert output.count(block) == 1
115+
assert output.index("before diff") < output.index(block) < output.index("after diff")
116+
assert console.consume_captured_output() == ""
117+
visible = capsys.readouterr().out
118+
assert visible.count(f"{fence}diff") == 1
119+
assert "[bold] [/invalid] :smile:" in visible
120+
if categorized:
121+
assert "Directly Modified:" in output
122+
assert "[bold]" not in output.split(block, 1)[1]
123+
124+
125+
@pytest.mark.parametrize("no_diff,empty_diff", [(True, False), (False, True)])
126+
def test_markdown_console_omitted_model_diffs(markdown_context_diff, mocker, no_diff, empty_diff):
127+
context_diff = markdown_context_diff("SELECT 1 AS [old]", "SELECT 2 AS [new]")
128+
text_diff = mocker.patch.object(ContextDiff, "text_diff", return_value="")
129+
console = MarkdownConsole()
130+
console.show_model_difference_summary(
131+
context_diff, EnvironmentNamingInfo(), None, no_diff=no_diff
132+
)
133+
output = console.consume_captured_output()
134+
assert "```" not in output
135+
assert "**Directly Modified:**" in output
136+
if empty_diff:
137+
text_diff.assert_called_once()
138+
else:
139+
text_diff.assert_not_called()
140+
141+
142+
@pytest.mark.parametrize("no_diff", [False, True])
143+
def test_markdown_console_environment_sql_diffs(markdown_context_diff, no_diff):
144+
context_diff = markdown_context_diff("SELECT 1", "SELECT 2")
145+
context_diff.previous_environment_statements = [
146+
EnvironmentStatements(
147+
before_all=["SELECT [old] FROM [schema].[table]"],
148+
after_all=["SELECT [old]"],
149+
python_env={"version": Executable.value(1)},
150+
)
151+
]
152+
context_diff.environment_statements = [
153+
EnvironmentStatements(
154+
before_all=["SELECT [new], ':smile: [/invalid]\\path\t' FROM [schema].[table]"],
155+
after_all=[f"SELECT '```\n{'long text ' * 20}\n```' AS [new]"],
156+
python_env={"version": Executable.value(2)},
157+
)
158+
]
159+
console = MarkdownConsole(width=35)
160+
console.show_environment_difference_summary(context_diff, no_diff=no_diff)
161+
output = console.consume_captured_output()
162+
assert "**Summary of differences from" in output and "`prod`:**" in output
163+
if no_diff:
164+
assert "Environment statements:" not in output
165+
assert "```" not in output
166+
return
167+
168+
assert "Environment statements:" in output
169+
for language, diff in context_diff.environment_statements_diff(include_python_env=True):
170+
if language == "sql":
171+
fence = "````" if "```" in diff else "```"
172+
assert f"{fence}diff\n{diff}\n{fence}\n" in output
173+
else:
174+
assert diff in output
175+
assert output.index("before_all:") < output.index("after_all:") < output.index("dependencies:")
176+
177+
178+
@pytest.mark.parametrize("indirect_only", [False, True])
179+
def test_markdown_console_model_change_categories(
180+
markdown_context_diff, make_snapshot, indirect_only
181+
):
182+
context_diff = markdown_context_diff("SELECT 1 AS [old]", "SELECT 2 AS [new]")
183+
current, previous = next(iter(context_diff.modified_snapshots.values()))
184+
child_model = load_sql_based_model(
185+
parse("MODEL (name test.child, dialect tsql); SELECT * FROM test.model")
186+
)
187+
old_child, new_child = [
188+
make_snapshot(child_model, nodes={parent.name: parent.model})
189+
for parent in (previous, current)
190+
]
191+
new_child.categorize_as(SnapshotChangeCategory.INDIRECT_NON_BREAKING)
192+
old_metadata, new_metadata = [
193+
make_snapshot(
194+
load_sql_based_model(
195+
parse(f"MODEL (name test.metadata, owner {owner}); SELECT 1 AS value")
196+
)
197+
)
198+
for owner in ("old_owner", "new_owner")
199+
]
200+
context_diff.modified_snapshots.update(
201+
{
202+
new_child.name: (new_child, old_child),
203+
new_metadata.name: (new_metadata, old_metadata),
204+
}
205+
)
206+
context_diff.snapshots.update(
207+
{new_child.snapshot_id: new_child, new_metadata.snapshot_id: new_metadata}
208+
)
209+
assert context_diff.indirectly_modified(new_child.name)
210+
assert context_diff.metadata_updated(new_metadata.name)
211+
if indirect_only:
212+
del context_diff.modified_snapshots[current.name]
213+
214+
console = MarkdownConsole()
215+
console.show_model_difference_summary(
216+
context_diff, EnvironmentNamingInfo(), None, no_diff=False
217+
)
218+
output = console.consume_captured_output()
219+
assert "**Indirectly Modified:**\n- `test.child` (Indirect Non-breaking)" in output
220+
assert "**Metadata Updated:**\n- `test.metadata`" in output
221+
if indirect_only:
222+
assert "**Directly Modified:**" not in output
223+
assert "```" not in output
224+
else:
225+
assert " Indirectly Modified Children:\n - `test.child`" in output
226+
assert output.index(" ```diff") < output.index(" Indirectly Modified Children:")
2227

3228

4229
def test_markdown_console_warning_block():

‎tests/integrations/github/cicd/test_github_controller.py‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
from sqlmesh.core import constants as c
1313
from sqlmesh.core.config import CategorizerConfig
1414
from sqlmesh.core.dialect import parse_one
15+
from sqlmesh.core.environment import EnvironmentNamingInfo, EnvironmentStatements
1516
from sqlmesh.core.model import SqlModel
17+
from sqlmesh.core.snapshot import SnapshotChangeCategory
1618
from sqlmesh.core.user import User, UserRole
1719
from sqlmesh.core.plan.definition import Plan
1820
from sqlmesh.core.linter.rule import RuleViolation
@@ -800,6 +802,60 @@ def test_get_plan_summary_includes_warnings_and_errors(
800802
) in summary
801803

802804

805+
def test_get_plan_summary_preserves_literal_sql(
806+
github_client, make_controller, make_snapshot, mocker
807+
):
808+
controller = make_controller(
809+
"tests/fixtures/github/pull_request_synchronized.json", github_client
810+
)
811+
context_diff = controller.prod_plan.context_diff
812+
old, new = [
813+
make_snapshot(
814+
SqlModel(
815+
name="test.model",
816+
dialect="tsql",
817+
query=parse_one(f"SELECT [{column}] FROM [schema].[table]", dialect="tsql"),
818+
)
819+
)
820+
for column in ("old_col", "new_col")
821+
]
822+
new.categorize_as(SnapshotChangeCategory.NON_BREAKING)
823+
context_diff.is_new_environment = False
824+
context_diff.added = set()
825+
context_diff.snapshots = {new.snapshot_id: new}
826+
context_diff.modified_snapshots = {new.name: (new, old)}
827+
context_diff.environment_statements = [
828+
EnvironmentStatements(
829+
before_all=["SELECT [before] FROM [schema].[table]"],
830+
after_all=["SELECT ':smile: [/invalid] ```' AS [after]"],
831+
python_env={},
832+
)
833+
]
834+
plan = mocker.Mock(
835+
spec=Plan,
836+
context_diff=context_diff,
837+
restatements={},
838+
missing_intervals=[],
839+
user_provided_flags={},
840+
environment_naming_info=EnvironmentNamingInfo(),
841+
)
842+
controller._console.log_success("stale output")
843+
github_client.reset_mock()
844+
845+
summary = controller.get_plan_summary(plan)
846+
847+
diff = context_diff.text_diff(new.name)
848+
assert (
849+
" ```diff\n" + "\n".join(f" {line}" for line in diff.split("\n")) + "\n ```\n" in summary
850+
)
851+
for _, diff in context_diff.environment_statements_diff():
852+
fence = "````" if "```" in diff else "```"
853+
assert f"{fence}diff\n{diff}\n{fence}\n" in summary
854+
assert "stale output" not in summary
855+
assert controller._console.consume_captured_output() == ""
856+
assert not github_client.mock_calls
857+
858+
803859
def test_get_pr_environment_summary_includes_warnings_and_errors(
804860
github_client, make_controller: t.Callable[..., GithubController]
805861
):

0 commit comments

Comments
 (0)