From d8ba36cf168a2ea45d58f57b7be7a3a17c478862 Mon Sep 17 00:00:00 2001 From: Iaroslav Zeigerman Date: Tue, 29 Jul 2025 16:11:04 -0700 Subject: [PATCH 1/4] Fix: Respect the project configuration if the forward-only suffix was not set in the bot's config --- sqlmesh/integrations/github/cicd/config.py | 8 +------- sqlmesh/integrations/github/cicd/controller.py | 12 +++++++----- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/sqlmesh/integrations/github/cicd/config.py b/sqlmesh/integrations/github/cicd/config.py index a287bf1af5..e4932d612e 100644 --- a/sqlmesh/integrations/github/cicd/config.py +++ b/sqlmesh/integrations/github/cicd/config.py @@ -33,9 +33,7 @@ class GithubCICDBotConfig(BaseConfig): pr_environment_name: t.Optional[str] = None pr_min_intervals: t.Optional[int] = None prod_branch_names_: t.Optional[str] = Field(default=None, alias="prod_branch_name") - forward_only_branch_suffix_: t.Optional[str] = Field( - default=None, alias="forward_only_branch_suffix" - ) + forward_only_branch_suffix: t.Optional[str] = None @model_validator(mode="before") @classmethod @@ -76,10 +74,6 @@ def skip_pr_backfill(self) -> bool: return True return self.skip_pr_backfill_ - @property - def forward_only_branch_suffix(self) -> str: - return self.forward_only_branch_suffix_ or "-forward-only" - FIELDS_FOR_ANALYTICS: t.ClassVar[t.Set[str]] = { "invalidate_environment_after_deploy", "enable_deploy_command", diff --git a/sqlmesh/integrations/github/cicd/controller.py b/sqlmesh/integrations/github/cicd/controller.py index cc1131deff..8506378d0e 100644 --- a/sqlmesh/integrations/github/cicd/controller.py +++ b/sqlmesh/integrations/github/cicd/controller.py @@ -478,11 +478,13 @@ def pr_targets_prod_branch(self) -> bool: return self._pull_request.base.ref in self.bot_config.prod_branch_names @property - def forward_only_plan(self) -> bool: - head_ref = self._pull_request.head.ref - if isinstance(head_ref, str): - return head_ref.endswith(self.bot_config.forward_only_branch_suffix) - return False + def forward_only_plan(self) -> t.Optional[bool]: + if self.bot_config.forward_only_branch_suffix is not None: + head_ref = self._pull_request.head.ref + if isinstance(head_ref, str): + return head_ref.endswith(self.bot_config.forward_only_branch_suffix) + return False + return None @classmethod def _append_output(cls, key: str, value: str) -> None: From 9095c072d4b71a1b358cb6e292b96cacbf9c74a8 Mon Sep 17 00:00:00 2001 From: Iaroslav Zeigerman Date: Tue, 29 Jul 2025 16:17:32 -0700 Subject: [PATCH 2/4] Revert "Fix: Respect the project configuration if the forward-only suffix was not set in the bot's config" This reverts commit d8ba36cf168a2ea45d58f57b7be7a3a17c478862. --- sqlmesh/integrations/github/cicd/config.py | 8 +++++++- sqlmesh/integrations/github/cicd/controller.py | 12 +++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/sqlmesh/integrations/github/cicd/config.py b/sqlmesh/integrations/github/cicd/config.py index e4932d612e..a287bf1af5 100644 --- a/sqlmesh/integrations/github/cicd/config.py +++ b/sqlmesh/integrations/github/cicd/config.py @@ -33,7 +33,9 @@ class GithubCICDBotConfig(BaseConfig): pr_environment_name: t.Optional[str] = None pr_min_intervals: t.Optional[int] = None prod_branch_names_: t.Optional[str] = Field(default=None, alias="prod_branch_name") - forward_only_branch_suffix: t.Optional[str] = None + forward_only_branch_suffix_: t.Optional[str] = Field( + default=None, alias="forward_only_branch_suffix" + ) @model_validator(mode="before") @classmethod @@ -74,6 +76,10 @@ def skip_pr_backfill(self) -> bool: return True return self.skip_pr_backfill_ + @property + def forward_only_branch_suffix(self) -> str: + return self.forward_only_branch_suffix_ or "-forward-only" + FIELDS_FOR_ANALYTICS: t.ClassVar[t.Set[str]] = { "invalidate_environment_after_deploy", "enable_deploy_command", diff --git a/sqlmesh/integrations/github/cicd/controller.py b/sqlmesh/integrations/github/cicd/controller.py index 8506378d0e..cc1131deff 100644 --- a/sqlmesh/integrations/github/cicd/controller.py +++ b/sqlmesh/integrations/github/cicd/controller.py @@ -478,13 +478,11 @@ def pr_targets_prod_branch(self) -> bool: return self._pull_request.base.ref in self.bot_config.prod_branch_names @property - def forward_only_plan(self) -> t.Optional[bool]: - if self.bot_config.forward_only_branch_suffix is not None: - head_ref = self._pull_request.head.ref - if isinstance(head_ref, str): - return head_ref.endswith(self.bot_config.forward_only_branch_suffix) - return False - return None + def forward_only_plan(self) -> bool: + head_ref = self._pull_request.head.ref + if isinstance(head_ref, str): + return head_ref.endswith(self.bot_config.forward_only_branch_suffix) + return False @classmethod def _append_output(cls, key: str, value: str) -> None: From 50be28ffc1649e95ee29e5b480f5d6a9f5604a5e Mon Sep 17 00:00:00 2001 From: Iaroslav Zeigerman Date: Tue, 29 Jul 2025 16:19:08 -0700 Subject: [PATCH 3/4] Fix: Respect the project configuration if the forward-only suffix was not set in the bot's config --- sqlmesh/integrations/github/cicd/controller.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sqlmesh/integrations/github/cicd/controller.py b/sqlmesh/integrations/github/cicd/controller.py index cc1131deff..dd5ee70e76 100644 --- a/sqlmesh/integrations/github/cicd/controller.py +++ b/sqlmesh/integrations/github/cicd/controller.py @@ -479,10 +479,11 @@ def pr_targets_prod_branch(self) -> bool: @property def forward_only_plan(self) -> bool: + default = self._context.config.plan.forward_only head_ref = self._pull_request.head.ref if isinstance(head_ref, str): - return head_ref.endswith(self.bot_config.forward_only_branch_suffix) - return False + return head_ref.endswith(self.bot_config.forward_only_branch_suffix) or default + return default @classmethod def _append_output(cls, key: str, value: str) -> None: From d241be471dd1ff0ba000f042d308c40be21f6ea2 Mon Sep 17 00:00:00 2001 From: Iaroslav Zeigerman Date: Tue, 29 Jul 2025 16:35:42 -0700 Subject: [PATCH 4/4] add test --- .../github/cicd/test_github_controller.py | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/integrations/github/cicd/test_github_controller.py b/tests/integrations/github/cicd/test_github_controller.py index f1be97ee20..a27f75f459 100644 --- a/tests/integrations/github/cicd/test_github_controller.py +++ b/tests/integrations/github/cicd/test_github_controller.py @@ -735,3 +735,46 @@ def test_pr_comment_deploy_indicator_includes_command_namespace( assert "To **apply** this PR's plan to prod, comment:\n - `/deploy`" not in comment assert "To **apply** this PR's plan to prod, comment:\n - `#SQLMesh/deploy`" in comment + + +def test_forward_only_config_falls_back_to_plan_config( + github_client, + make_controller: t.Callable[..., GithubController], + mocker: MockerFixture, +): + mock_repo = github_client.get_repo() + mock_repo.create_check_run = mocker.MagicMock( + side_effect=lambda **kwargs: make_mock_check_run(**kwargs) + ) + + created_comments = [] + mock_issue = mock_repo.get_issue() + mock_issue.create_comment = mocker.MagicMock( + side_effect=lambda comment: make_mock_issue_comment( + comment=comment, created_comments=created_comments + ) + ) + mock_issue.get_comments = mocker.MagicMock(side_effect=lambda: created_comments) + + mock_pull_request = mock_repo.get_pull() + mock_pull_request.get_reviews = mocker.MagicMock(lambda: []) + mock_pull_request.merged = False + mock_pull_request.merge = mocker.MagicMock() + mock_pull_request.head.ref = "unit-test-test-pr" + + controller = make_controller( + "tests/fixtures/github/pull_request_synchronized.json", + github_client, + bot_config=GithubCICDBotConfig( + merge_method=MergeMethod.SQUASH, + enable_deploy_command=True, + forward_only_branch_suffix="-forward-only", + ), + mock_out_context=False, + ) + + controller._context.config.plan.forward_only = True + assert controller.forward_only_plan + + controller._context.config.plan.forward_only = False + assert controller.forward_only_plan is False