From 5ed80829b35e0f5fc9227a98d65924f32960ee28 Mon Sep 17 00:00:00 2001 From: Erin Drummond Date: Mon, 7 Jul 2025 02:53:40 +0000 Subject: [PATCH 1/2] Fix(cicd_bot): Don't truncate backfill model list --- sqlmesh/core/console.py | 4 +- sqlmesh/integrations/github/cicd/command.py | 4 +- .../integrations/github/cicd/controller.py | 9 +++ .../github/cicd/test_github_controller.py | 38 ++++++++++++ .../github/cicd/test_integration.py | 62 +++++++++---------- 5 files changed, 84 insertions(+), 33 deletions(-) diff --git a/sqlmesh/core/console.py b/sqlmesh/core/console.py index ae48722288..78020b693f 100644 --- a/sqlmesh/core/console.py +++ b/sqlmesh/core/console.py @@ -3107,7 +3107,9 @@ class MarkdownConsole(CaptureTerminalConsole): AUDIT_PADDING = 7 def __init__(self, **kwargs: t.Any) -> None: - super().__init__(**{**kwargs, "console": RichConsole(no_color=True)}) + super().__init__( + **{**kwargs, "console": RichConsole(no_color=True, width=kwargs.pop("width", None))} + ) def show_environment_difference_summary( self, diff --git a/sqlmesh/integrations/github/cicd/command.py b/sqlmesh/integrations/github/cicd/command.py index f5d614405a..04f5757b50 100644 --- a/sqlmesh/integrations/github/cicd/command.py +++ b/sqlmesh/integrations/github/cicd/command.py @@ -28,7 +28,9 @@ @click.pass_context 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_console(MarkdownConsole()) + # 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)) 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 5ae2a763e7..15ec52781a 100644 --- a/sqlmesh/integrations/github/cicd/controller.py +++ b/sqlmesh/integrations/github/cicd/controller.py @@ -485,6 +485,12 @@ def _append_output(cls, key: str, value: str) -> None: print(f"{key}={value}", file=fh) def get_plan_summary(self, plan: Plan) -> str: + # use Verbosity.VERY_VERBOSE to prevent the list of models from being truncated + # this is particularly importanmt for the "Models needing backfill" list because + # there is no easy way to tell this otherwise + orig_verbosity = self._console.verbosity + self._console.verbosity = Verbosity.VERY_VERBOSE + try: # Clear out any output that might exist from prior steps self._console.clear_captured_outputs() @@ -517,7 +523,10 @@ def get_plan_summary(self, plan: Plan) -> str: return f"{difference_summary}\n{missing_dates}{plan_flags_section}" except PlanError as e: + logger.exception("Plan failed to generate") return f"Plan failed to generate. Check for pending or unresolved changes. Error: {e}" + finally: + self._console.verbosity = orig_verbosity def run_tests(self) -> t.Tuple[ModelTextTestResult, str]: """ diff --git a/tests/integrations/github/cicd/test_github_controller.py b/tests/integrations/github/cicd/test_github_controller.py index d7d4f5343c..62cd5a26f4 100644 --- a/tests/integrations/github/cicd/test_github_controller.py +++ b/tests/integrations/github/cicd/test_github_controller.py @@ -1,4 +1,5 @@ # type: ignore +import typing as t import os import pathlib from unittest import mock @@ -17,6 +18,7 @@ BotCommand, MergeStateStatus, ) +from sqlmesh.integrations.github.cicd.controller import GithubController from sqlmesh.integrations.github.cicd.command import _update_pr_environment from sqlmesh.utils.date import to_datetime, now from tests.integrations.github.cicd.conftest import MockIssueComment @@ -591,3 +593,39 @@ def test_uncategorized( assert "The following models could not be categorized automatically" in summary assert '- "b"' in summary assert "Run `sqlmesh plan hello_world_2` locally to apply these changes" in summary + + +def test_get_plan_summary_doesnt_truncate_backfill_list( + github_client, make_controller: t.Callable[..., GithubController] +): + controller = make_controller( + "tests/fixtures/github/pull_request_synchronized.json", + github_client, + mock_out_context=False, + ) + + summary = controller.get_plan_summary(controller.prod_plan) + + assert "more ...." not in summary + + assert ( + """**Models needing backfill:** +* `memory.raw.demographics`: [full refresh] +* `memory.sushi.active_customers`: [full refresh] +* `memory.sushi.count_customers_active`: [full refresh] +* `memory.sushi.count_customers_inactive`: [full refresh] +* `memory.sushi.customer_revenue_by_day`: [2025-06-30 - 2025-07-06] +* `memory.sushi.customer_revenue_lifetime`: [2025-06-30 - 2025-07-06] +* `memory.sushi.customers`: [full refresh] +* `memory.sushi.items`: [2025-06-30 - 2025-07-06] +* `memory.sushi.latest_order`: [full refresh] +* `memory.sushi.marketing`: [2025-06-30 - 2025-07-06] +* `memory.sushi.order_items`: [2025-06-30 - 2025-07-06] +* `memory.sushi.orders`: [2025-06-30 - 2025-07-06] +* `memory.sushi.raw_marketing`: [full refresh] +* `memory.sushi.top_waiters`: [recreate view] +* `memory.sushi.waiter_as_customer_by_day`: [2025-06-30 - 2025-07-06] +* `memory.sushi.waiter_names`: [full refresh] +* `memory.sushi.waiter_revenue_by_day`: [2025-06-30 - 2025-07-06]""" + in summary + ) diff --git a/tests/integrations/github/cicd/test_integration.py b/tests/integrations/github/cicd/test_integration.py index bff9d4d117..ff9856993a 100644 --- a/tests/integrations/github/cicd/test_integration.py +++ b/tests/integrations/github/cicd/test_integration.py @@ -298,7 +298,7 @@ def test_merge_pr_has_non_breaking_change( assert GithubCheckConclusion(prod_plan_preview_checks_runs[2]["conclusion"]).is_success expected_prod_plan_directly_modified_summary = """**Directly Modified:** -* `sushi.waiter_revenue_by_day` (Non-breaking) +* `memory.sushi.waiter_revenue_by_day` (Non-breaking) ```diff --- @@ -318,10 +318,10 @@ def test_merge_pr_has_non_breaking_change( ON o.id = oi.order_id AND o.event_date = oi.event_date ``` Indirectly Modified Children: - - `sushi.top_waiters` (Indirect Non-breaking) + - `memory.sushi.top_waiters` (Indirect Non-breaking) """ expected_prod_plan_indirectly_modified_summary = """**Indirectly Modified:** -- `sushi.top_waiters` (Indirect Non-breaking) +- `memory.sushi.top_waiters` (Indirect Non-breaking) """ assert prod_plan_preview_checks_runs[2]["output"]["title"] == "Prod Plan Preview" @@ -509,7 +509,7 @@ def test_merge_pr_has_non_breaking_change_diff_start( assert prod_plan_preview_checks_runs[2]["output"]["title"] == "Prod Plan Preview" expected_prod_plan_directly_modified_summary = """**Directly Modified:** -* `sushi.waiter_revenue_by_day` (Non-breaking) +* `memory.sushi.waiter_revenue_by_day` (Non-breaking) ```diff --- @@ -529,10 +529,10 @@ def test_merge_pr_has_non_breaking_change_diff_start( ON o.id = oi.order_id AND o.event_date = oi.event_date ``` Indirectly Modified Children: - - `sushi.top_waiters` (Indirect Non-breaking) + - `memory.sushi.top_waiters` (Indirect Non-breaking) """ expected_prod_plan_indirectly_modified_summary = """**Indirectly Modified:** -- `sushi.top_waiters` (Indirect Non-breaking) +- `memory.sushi.top_waiters` (Indirect Non-breaking) """ prod_plan_preview_summary = prod_plan_preview_checks_runs[2]["output"]["summary"] @@ -1032,7 +1032,7 @@ def test_no_merge_since_no_deploy_signal( assert GithubCheckConclusion(prod_plan_preview_checks_runs[2]["conclusion"]).is_success expected_prod_plan_directly_modified_summary = """**Directly Modified:** -* `sushi.waiter_revenue_by_day` (Non-breaking) +* `memory.sushi.waiter_revenue_by_day` (Non-breaking) ```diff --- @@ -1052,10 +1052,10 @@ def test_no_merge_since_no_deploy_signal( ON o.id = oi.order_id AND o.event_date = oi.event_date ``` Indirectly Modified Children: - - `sushi.top_waiters` (Indirect Non-breaking)""" + - `memory.sushi.top_waiters` (Indirect Non-breaking)""" expected_prod_plan_indirectly_modified_summary = """**Indirectly Modified:** -- `sushi.top_waiters` (Indirect Non-breaking) +- `memory.sushi.top_waiters` (Indirect Non-breaking) """ assert prod_plan_preview_checks_runs[2]["output"]["title"] == "Prod Plan Preview" @@ -1232,7 +1232,7 @@ def test_no_merge_since_no_deploy_signal_no_approvers_defined( assert GithubCheckStatus(prod_plan_preview_checks_runs[2]["status"]).is_completed assert GithubCheckConclusion(prod_plan_preview_checks_runs[2]["conclusion"]).is_success expected_prod_plan_directly_modified_summary = """**Directly Modified:** -* `sushi.waiter_revenue_by_day` (Non-breaking) +* `memory.sushi.waiter_revenue_by_day` (Non-breaking) ```diff --- @@ -1252,10 +1252,10 @@ def test_no_merge_since_no_deploy_signal_no_approvers_defined( ON o.id = oi.order_id AND o.event_date = oi.event_date ``` Indirectly Modified Children: - - `sushi.top_waiters` (Indirect Non-breaking) + - `memory.sushi.top_waiters` (Indirect Non-breaking) """ expected_prod_plan_indirectly_modified_summary = """**Indirectly Modified:** -- `sushi.top_waiters` (Indirect Non-breaking) +- `memory.sushi.top_waiters` (Indirect Non-breaking) """ assert prod_plan_preview_checks_runs[2]["output"]["title"] == "Prod Plan Preview" prod_plan_preview_summary = prod_plan_preview_checks_runs[2]["output"]["summary"] @@ -1414,7 +1414,7 @@ def test_deploy_comment_pre_categorized( assert GithubCheckStatus(prod_plan_preview_checks_runs[2]["status"]).is_completed assert GithubCheckConclusion(prod_plan_preview_checks_runs[2]["conclusion"]).is_success expected_prod_plan_directly_modified_summary = """**Directly Modified:** -* `sushi.waiter_revenue_by_day` (Non-breaking) +* `memory.sushi.waiter_revenue_by_day` (Non-breaking) ```diff --- @@ -1434,10 +1434,10 @@ def test_deploy_comment_pre_categorized( ON o.id = oi.order_id AND o.event_date = oi.event_date ``` Indirectly Modified Children: - - `sushi.top_waiters` (Indirect Non-breaking) + - `memory.sushi.top_waiters` (Indirect Non-breaking) """ expected_prod_plan_indirectly_modified_summary = """**Indirectly Modified:** -- `sushi.top_waiters` (Indirect Non-breaking) +- `memory.sushi.top_waiters` (Indirect Non-breaking) """ assert prod_plan_preview_checks_runs[2]["output"]["title"] == "Prod Plan Preview" prod_plan_preview_summary = prod_plan_preview_checks_runs[2]["output"]["summary"] @@ -1781,7 +1781,7 @@ def test_overlapping_changes_models( assert GithubCheckConclusion(prod_plan_preview_checks_runs[2]["conclusion"]).is_success expected_prod_plan_directly_modified_summary = """**Directly Modified:** -* `sushi.customers` (Non-breaking) +* `memory.sushi.customers` (Non-breaking) ```diff --- @@ -1801,23 +1801,23 @@ def test_overlapping_changes_models( WITH current_marketing AS ( ``` Indirectly Modified Children: - - `sushi.active_customers` (Indirect Non-breaking) - - `sushi.count_customers_active` (Indirect Non-breaking) - - `sushi.count_customers_inactive` (Indirect Non-breaking) - - `sushi.waiter_as_customer_by_day` (Indirect Breaking) + - `memory.sushi.active_customers` (Indirect Non-breaking) + - `memory.sushi.count_customers_active` (Indirect Non-breaking) + - `memory.sushi.count_customers_inactive` (Indirect Non-breaking) + - `memory.sushi.waiter_as_customer_by_day` (Indirect Breaking) -* `sushi.waiter_names` (Breaking) +* `memory.sushi.waiter_names` (Breaking) Indirectly Modified Children: - - `sushi.waiter_as_customer_by_day` (Indirect Breaking)""" + - `memory.sushi.waiter_as_customer_by_day` (Indirect Breaking)""" expected_prod_plan_indirectly_modified_summary = """**Indirectly Modified:** -- `sushi.active_customers` (Indirect Non-breaking) -- `sushi.count_customers_active` (Indirect Non-breaking) -- `sushi.count_customers_inactive` (Indirect Non-breaking) -- `sushi.waiter_as_customer_by_day` (Indirect Breaking)""" +- `memory.sushi.active_customers` (Indirect Non-breaking) +- `memory.sushi.count_customers_active` (Indirect Non-breaking) +- `memory.sushi.count_customers_inactive` (Indirect Non-breaking) +- `memory.sushi.waiter_as_customer_by_day` (Indirect Breaking)""" assert prod_plan_preview_checks_runs[2]["output"]["title"] == "Prod Plan Preview" prod_plan_preview_summary = prod_plan_preview_checks_runs[2]["output"]["summary"] @@ -1993,7 +1993,7 @@ def test_pr_add_model( ) expected_prod_plan_summary = """**Added Models:** -- `sushi.cicd_test_model` (Breaking)""" +- `memory.sushi.cicd_test_model` (Breaking)""" assert "SQLMesh - Prod Plan Preview" in controller._check_run_mapping prod_plan_preview_checks_runs = controller._check_run_mapping[ @@ -2144,7 +2144,7 @@ def test_pr_delete_model( ) expected_prod_plan_summary = """**Removed Models:** -- `sushi.top_waiters` (Breaking)""" +- `memory.sushi.top_waiters` (Breaking)""" assert "SQLMesh - Prod Plan Preview" in controller._check_run_mapping prod_plan_preview_checks_runs = controller._check_run_mapping[ @@ -2330,7 +2330,7 @@ def test_has_required_approval_but_not_base_branch( assert GithubCheckStatus(prod_plan_preview_checks_runs[2]["status"]).is_completed assert GithubCheckConclusion(prod_plan_preview_checks_runs[2]["conclusion"]).is_success expected_prod_plan_directly_modified_summary = """**Directly Modified:** -* `sushi.waiter_revenue_by_day` (Non-breaking) +* `memory.sushi.waiter_revenue_by_day` (Non-breaking) ```diff --- @@ -2350,10 +2350,10 @@ def test_has_required_approval_but_not_base_branch( ON o.id = oi.order_id AND o.event_date = oi.event_date ``` Indirectly Modified Children: - - `sushi.top_waiters` (Indirect Non-breaking)""" + - `memory.sushi.top_waiters` (Indirect Non-breaking)""" expected_prod_plan_indirectly_modified_summary = """**Indirectly Modified:** -- `sushi.top_waiters` (Indirect Non-breaking)""" +- `memory.sushi.top_waiters` (Indirect Non-breaking)""" assert prod_plan_preview_checks_runs[2]["output"]["title"] == "Prod Plan Preview" prod_plan_preview_summary = prod_plan_preview_checks_runs[2]["output"]["summary"] From 2a82b280400b5a7976d8959b9abca330623e78e2 Mon Sep 17 00:00:00 2001 From: Erin Drummond Date: Mon, 7 Jul 2025 20:18:05 +0000 Subject: [PATCH 2/2] PR feedback --- sqlmesh/integrations/github/cicd/controller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlmesh/integrations/github/cicd/controller.py b/sqlmesh/integrations/github/cicd/controller.py index 15ec52781a..bc35e54520 100644 --- a/sqlmesh/integrations/github/cicd/controller.py +++ b/sqlmesh/integrations/github/cicd/controller.py @@ -486,7 +486,7 @@ def _append_output(cls, key: str, value: str) -> None: def get_plan_summary(self, plan: Plan) -> str: # use Verbosity.VERY_VERBOSE to prevent the list of models from being truncated - # this is particularly importanmt for the "Models needing backfill" list because + # this is particularly important for the "Models needing backfill" list because # there is no easy way to tell this otherwise orig_verbosity = self._console.verbosity self._console.verbosity = Verbosity.VERY_VERBOSE