fix!: check for base branch when deploying to prod

Author: eakmanrq

docs/integrations/github.md

@@ -113,6 +113,7 @@ In this example we configured the merge method to be `squash`. See [Bot Configur
 
 One way to signal to SQLMesh that a PR is ready to go to production is through the use of "Required Approvers". 
 In this approach users configure their SQLMesh project to list users that are designated as "Required Approver" and then when the bot detects an approval was received from one of these individuals then it determines that it is time to deploy to production.
+The bot will only do the deploy to prod if the base branch is a production branch (as defined in the bot's configuration but defaults to either `main` or `master`).
 This pattern can be a great fit for teams that already have an approval process like this in place and therefore it actually removes an extra step from either the author or the approver since SQLMesh will automate the deployment and merge until of it having to be manually done.
 
 ##### Required Approval Configuration
@@ -295,8 +296,9 @@ Below is an example of how to define the default config for the bot in either YA
 | `default_pr_start`                    | Default start when creating PR environment plans. If running in a mode where the bot automatically backfills models (based on `auto_categorize_changes` behavior) then this can be used to limit the amount of data backfilled. Defaults to `None` meaning the start date is set to the earliest model's start or to 1 day ago if [data previews](../concepts/plans.md#data-preview) need to be computed. |  str   |    N     |
 | `skip_pr_backfill`                    | Indicates if the bot should skip backfilling models in the PR environment. Default: `True`                                                                                                                                                                                                                                                                                                                |  bool  |    N     |
 | `pr_include_unmodified`               | Indicates whether to include unmodified models in the PR environment. Default to the project's config value (which defaults to `False`)                                                                                                                                                                                                                                                                   |  bool  |    N     |
-| `run_on_deploy_to_prod`               | Indicates whether to run latest intervals when deploying to prod. If set to false, the deployment will backfill only the changed models up to the existing latest interval in production, ignoring any missing intervals beyond this point. Default: `False`                                                                                                                                               |  bool  |    N     |
-| `pr_environment_name`                 | The name of the PR environment to create for which a PR number will be appended to. Defaults to the repo name if not provided. Note: The name will be normalized to alphanumeric + underscore and lowercase.                                                                                                                                                                                              |  str   |    N     |	
+| `run_on_deploy_to_prod`               | Indicates whether to run latest intervals when deploying to prod. If set to false, the deployment will backfill only the changed models up to the existing latest interval in production, ignoring any missing intervals beyond this point. Default: `False`                                                                                                                                              |  bool  |    N     |
+| `pr_environment_name`                 | The name of the PR environment to create for which a PR number will be appended to. Defaults to the repo name if not provided. Note: The name will be normalized to alphanumeric + underscore and lowercase.                                                                                                                                                                                              |  str   |    N     |
+| `prod_branch_name`                    | The name of the git branch associated with production. Ex: "prod". Default: "main" or "master" is considered prod                                                                                                                                                                                                                                                                                         |  str   |    N     |
 
 Example with all properties defined:
 
@@ -317,6 +319,7 @@ Example with all properties defined:
       default_pr_start: "1 week ago"
       skip_pr_backfill: false
       run_on_deploy_to_prod: false
+      prod_branch_name: production
     ```
 
 === "Python"
@@ -340,6 +343,7 @@ Example with all properties defined:
             default_pr_start="1 week ago",
             skip_pr_backfill=False,
             run_on_deploy_to_prod=False,
+            prod_branch_name="production",
         )
     )
     ```

sqlmesh/integrations/github/cicd/command.py

@@ -210,7 +210,7 @@ def _run_all(controller: GithubController) -> None:
     has_required_approval = False
     is_auto_deploying_prod = (
         controller.deploy_command_enabled or controller.do_required_approval_check
-    )
+    ) and controller.pr_targets_prod_branch
     if controller.is_comment_added:
         if not controller.deploy_command_enabled:
             # We aren't using commands so we can just return
@@ -267,7 +267,7 @@ def _run_all(controller: GithubController) -> None:
             status=GithubCheckStatus.COMPLETED, conclusion=GithubCheckConclusion.SKIPPED
         )
     deployed_to_prod = False
-    if has_required_approval and prod_plan_generated:
+    if has_required_approval and prod_plan_generated and controller.pr_targets_prod_branch:
         deployed_to_prod = _deploy_production(controller)
     elif is_auto_deploying_prod:
         if not has_required_approval:
@@ -292,7 +292,7 @@ def _run_all(controller: GithubController) -> None:
     if (
         not pr_environment_updated
         or not prod_plan_generated
-        or (has_required_approval and not deployed_to_prod)
+        or (has_required_approval and controller.pr_targets_prod_branch and not deployed_to_prod)
     ):
         raise CICDBotError(
             "A step of the run-all check failed. See check status for more information."

sqlmesh/integrations/github/cicd/config.py

@@ -28,6 +28,7 @@ class GithubCICDBotConfig(BaseConfig):
     pr_include_unmodified: t.Optional[bool] = None
     run_on_deploy_to_prod: bool = False
     pr_environment_name: t.Optional[str] = None
+    prod_branch_names_: t.Optional[str] = Field(default=None, alias="prod_branch_name")
 
     @model_validator(mode="before")
     @classmethod
@@ -42,6 +43,12 @@ def _validate(cls, data: t.Any) -> t.Any:
 
         return data
 
+    @property
+    def prod_branch_names(self) -> t.List[str]:
+        if self.prod_branch_names_:
+            return [self.prod_branch_names_]
+        return ["main", "master"]
+
     FIELDS_FOR_ANALYTICS: t.ClassVar[t.Set[str]] = {
         "invalidate_environment_after_deploy",
         "enable_deploy_command",

sqlmesh/integrations/github/cicd/controller.py

@@ -76,7 +76,7 @@ def create_from_pull_request_url(cls, pull_request_url: str) -> PullRequestInfo:
         return cls(
             owner=owner,
             repo=repo,
-            pr_number=pr_number,
+            pr_number=int(pr_number),
         )
 
 
@@ -444,6 +444,10 @@ def modified_snapshots(self) -> t.Dict[SnapshotId, t.Union[Snapshot, SnapshotTab
     def removed_snapshots(self) -> t.Set[SnapshotId]:
         return set(self.prod_plan_with_gaps.context_diff.removed_snapshots)
 
+    @property
+    def pr_targets_prod_branch(self) -> bool:
+        return self._pull_request.base.ref in self.bot_config.prod_branch_names
+
     @classmethod
     def _append_output(cls, key: str, value: str) -> None:
         """

tests/integrations/github/cicd/fixtures.py

@@ -25,14 +25,15 @@ def github_client(mocker: MockerFixture):
 
     client_mock = mocker.MagicMock(spec=Github)
     mocker.patch("github.Github", client_mock)
+
     mock_repository = mocker.MagicMock(spec=Repository)
+    client_mock.get_repo.return_value = mock_repository
+
     mock_pull_request = mocker.MagicMock(spec=PullRequest)
-    mock_pull_request.get_reviews = mocker.MagicMock(
-        side_effect=[mocker.MagicMock(spec=PullRequestReview)]
-    )
-    mock_repository.get_pull = mocker.MagicMock(side_effect=mock_pull_request)
-    mock_repository.get_issue = mocker.MagicMock(side_effect=mocker.MagicMock(spec=Issue))
-    client_mock.get_repo = mocker.MagicMock(side_effect=mock_repository)
+    mock_pull_request.base.ref = "main"
+    mock_pull_request.get_reviews.return_value = [mocker.MagicMock(spec=PullRequestReview)]
+    mock_repository.get_pull.return_value = mock_pull_request
+    mock_repository.get_issue.return_value = mocker.MagicMock(spec=Issue)
 
     return client_mock
 

tests/integrations/github/cicd/test_config.py

@@ -40,6 +40,7 @@ def test_load_yaml_config_default(tmp_path):
     assert config.cicd_bot.skip_pr_backfill
     assert config.cicd_bot.pr_include_unmodified is None
     assert config.cicd_bot.pr_environment_name is None
+    assert config.cicd_bot.prod_branch_names == ["main", "master"]
 
 
 def test_load_yaml_config(tmp_path):
@@ -62,6 +63,7 @@ def test_load_yaml_config(tmp_path):
     skip_pr_backfill: false
     pr_include_unmodified: true
     pr_environment_name: "MyOverride"
+    prod_branch_name: testing
 model_defaults:
     dialect: duckdb
 """,
@@ -85,6 +87,7 @@ def test_load_yaml_config(tmp_path):
     assert not config.cicd_bot.skip_pr_backfill
     assert config.cicd_bot.pr_include_unmodified
     assert config.cicd_bot.pr_environment_name == "MyOverride"
+    assert config.cicd_bot.prod_branch_names == ["testing"]
 
 
 def test_load_python_config_defaults(tmp_path):
@@ -115,6 +118,7 @@ def test_load_python_config_defaults(tmp_path):
     assert config.cicd_bot.skip_pr_backfill
     assert config.cicd_bot.pr_include_unmodified is None
     assert config.cicd_bot.pr_environment_name is None
+    assert config.cicd_bot.prod_branch_names == ["main", "master"]
 
 
 def test_load_python_config(tmp_path):
@@ -141,6 +145,7 @@ def test_load_python_config(tmp_path):
         skip_pr_backfill=False,
         pr_include_unmodified=True,
         pr_environment_name="MyOverride",
+        prod_branch_name="testing",
     ),
     model_defaults=ModelDefaultsConfig(dialect="duckdb"),
 )
@@ -166,6 +171,7 @@ def test_load_python_config(tmp_path):
     assert not config.cicd_bot.skip_pr_backfill
     assert config.cicd_bot.pr_include_unmodified
     assert config.cicd_bot.pr_environment_name == "MyOverride"
+    assert config.cicd_bot.prod_branch_names == ["testing"]
 
 
 def test_validation(tmp_path):

tests/integrations/github/cicd/test_integration.py

@@ -1948,3 +1948,182 @@ def test_pr_delete_model(
             output
             == "run_unit_tests=success\nhas_required_approval=success\ncreated_pr_environment=true\npr_environment_name=hello_world_2\npr_environment_synced=success\nprod_plan_preview=success\nprod_environment_synced=success\n"
         )
+
+
+@time_machine.travel("2023-01-01 15:00:00 UTC")
+def test_has_required_approval_but_not_base_branch(
+    github_client,
+    make_controller,
+    make_mock_check_run,
+    make_mock_issue_comment,
+    make_pull_request_review,
+    tmp_path: pathlib.Path,
+    mocker: MockerFixture,
+):
+    """
+    PR with a non-breaking change and auto-categorization will be backfilled, but NOT automatically merged or deployed to production if it is branched from a non-production branch.
+
+    Scenario:
+    - PR is not merged
+    - PR has been approved by a required reviewer
+    - Tests passed
+    - PR Merge Method defined
+    - Delete environment is disabled
+    - Changes made in PR with auto-categorization
+    - PR is not merged, despite having required approval, since the base branch is not prod
+    """
+    mock_repo = github_client.get_repo()
+    mock_repo.create_check_run = mocker.MagicMock(
+        side_effect=lambda **kwargs: make_mock_check_run(**kwargs)
+    )
+
+    created_comments: t.List[MockIssueComment] = []
+    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.base.ref = "feature/branch"
+    mock_pull_request.get_reviews = mocker.MagicMock(
+        side_effect=lambda: [make_pull_request_review(username="test_github", state="APPROVED")]
+    )
+    mock_pull_request.merged = False
+    mock_pull_request.merge = mocker.MagicMock()
+
+    controller = make_controller(
+        "tests/fixtures/github/pull_request_synchronized.json",
+        github_client,
+        bot_config=GithubCICDBotConfig(
+            merge_method=MergeMethod.MERGE,
+            invalidate_environment_after_deploy=False,
+            auto_categorize_changes=CategorizerConfig.all_full(),
+            default_pr_start=None,
+            skip_pr_backfill=False,
+        ),
+        mock_out_context=False,
+    )
+    controller._context.plan("prod", no_prompts=True, auto_apply=True)
+    controller._context.users = [
+        User(username="test", github_username="test_github", roles=[UserRole.REQUIRED_APPROVER])
+    ]
+    # Make a non-breaking change
+    model = controller._context.get_model("sushi.waiter_revenue_by_day").copy()
+    model.query.select(exp.alias_("1", "new_col"), copy=False)
+    controller._context.upsert_model(model)
+
+    github_output_file = tmp_path / "github_output.txt"
+
+    with mock.patch.dict(os.environ, {"GITHUB_OUTPUT": str(github_output_file)}):
+        command._run_all(controller)
+
+    assert "SQLMesh - Run Unit Tests" in controller._check_run_mapping
+    test_checks_runs = controller._check_run_mapping["SQLMesh - Run Unit Tests"].all_kwargs
+    assert len(test_checks_runs) == 3
+    assert GithubCheckStatus(test_checks_runs[0]["status"]).is_queued
+    assert GithubCheckStatus(test_checks_runs[1]["status"]).is_in_progress
+    assert GithubCheckStatus(test_checks_runs[2]["status"]).is_completed
+    assert GithubCheckConclusion(test_checks_runs[2]["conclusion"]).is_success
+    assert test_checks_runs[2]["output"]["title"] == "Tests Passed"
+    assert (
+        test_checks_runs[2]["output"]["summary"].strip()
+        == "**Successfully Ran `3` Tests Against `duckdb`**"
+    )
+
+    assert "SQLMesh - PR Environment Synced" in controller._check_run_mapping
+    pr_checks_runs = controller._check_run_mapping["SQLMesh - PR Environment Synced"].all_kwargs
+    assert len(pr_checks_runs) == 3
+    assert GithubCheckStatus(pr_checks_runs[0]["status"]).is_queued
+    assert GithubCheckStatus(pr_checks_runs[1]["status"]).is_in_progress
+    assert GithubCheckStatus(pr_checks_runs[2]["status"]).is_completed
+    assert GithubCheckConclusion(pr_checks_runs[2]["conclusion"]).is_success
+    assert pr_checks_runs[2]["output"]["title"] == "PR Virtual Data Environment: hello_world_2"
+    assert (
+        pr_checks_runs[2]["output"]["summary"]
+        == """<table><thead><tr><th colspan="3">PR Environment Summary</th></tr><tr><th>Model</th><th>Change Type</th><th>Dates Loaded</th></tr></thead><tbody><tr><td>sushi.waiter_revenue_by_day</td><td>Non-breaking</td><td>2022-12-25 - 2022-12-31</td></tr></tbody></table>"""
+    )
+
+    assert "SQLMesh - Prod Plan Preview" in controller._check_run_mapping
+    prod_plan_preview_checks_runs = controller._check_run_mapping[
+        "SQLMesh - Prod Plan Preview"
+    ].all_kwargs
+    assert len(prod_plan_preview_checks_runs) == 3
+    assert GithubCheckStatus(prod_plan_preview_checks_runs[0]["status"]).is_queued
+    assert GithubCheckStatus(prod_plan_preview_checks_runs[1]["status"]).is_in_progress
+    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_summary = """\n**Summary of differences from `prod`:**
+
+**Directly Modified:**
+- `sushi.waiter_revenue_by_day`
+```diff
+--- 
+
++++ 
+
+@@ -16,7 +16,8 @@
+
+ SELECT
+   CAST(o.waiter_id AS INT) AS waiter_id,
+   CAST(SUM(oi.quantity * i.price) AS DOUBLE) AS revenue,
+-  CAST(o.event_date AS DATE) AS event_date
++  CAST(o.event_date AS DATE) AS event_date,
++  1 AS new_col
+ FROM sushi.orders AS o
+ LEFT JOIN sushi.order_items AS oi
+   ON o.id = oi.order_id AND o.event_date = oi.event_date
+```
+
+**Indirectly Modified:**
+- `sushi.top_waiters`
+
+"""
+    assert prod_plan_preview_checks_runs[2]["output"]["title"] == "Prod Plan Preview"
+    assert prod_plan_preview_checks_runs[2]["output"]["summary"] == expected_prod_plan_summary
+
+    assert "SQLMesh - Prod Environment Synced" not in controller._check_run_mapping
+
+    assert "SQLMesh - Has Required Approval" in controller._check_run_mapping
+    approval_checks_runs = controller._check_run_mapping[
+        "SQLMesh - Has Required Approval"
+    ].all_kwargs
+    assert len(approval_checks_runs) == 3
+    assert GithubCheckStatus(approval_checks_runs[0]["status"]).is_queued
+    assert GithubCheckStatus(approval_checks_runs[1]["status"]).is_in_progress
+    assert GithubCheckStatus(approval_checks_runs[2]["status"]).is_completed
+    assert GithubCheckConclusion(approval_checks_runs[2]["conclusion"]).is_success
+    assert (
+        approval_checks_runs[2]["output"]["title"]
+        == "Obtained approval from required approvers: test_github"
+    )
+    assert (
+        approval_checks_runs[2]["output"]["summary"]
+        == """**List of possible required approvers:**
+- `test_github`
+"""
+    )
+
+    assert len(get_environment_objects(controller, "hello_world_2")) == 2
+    assert get_num_days_loaded(controller, "hello_world_2", "waiter_revenue_by_day") == 7
+    assert "new_col" in get_columns(controller, "hello_world_2", "waiter_revenue_by_day")
+    assert "new_col" not in get_columns(controller, None, "waiter_revenue_by_day")
+
+    assert not mock_pull_request.merge.called
+
+    assert len(created_comments) == 1
+    assert (
+        created_comments[0].body
+        == """:robot: **SQLMesh Bot Info** :robot:
+- :eyes: To **review** this PR's changes, use virtual data environment:
+  - `hello_world_2`"""
+    )
+
+    with open(github_output_file, "r", encoding="utf-8") as f:
+        output = f.read()
+        assert (
+            output
+            == "run_unit_tests=success\nhas_required_approval=success\ncreated_pr_environment=true\npr_environment_name=hello_world_2\npr_environment_synced=success\nprod_plan_preview=success\n"
+        )