Repository: TobikoData/sqlmesh
Pull request #4484 (2025-05-20 21:50:39)

fix!: check for base branch when deploying to prod
Currently if a user branches off a non-prod branch and then has a required approver approve, the bot will deploy and merge the branch like it is going to production. This PR updates the behavior so it checks if the base branch is prod and if not then it will not do the prod deploy. 

It is technically breaking since if a user currently has a prod branch name that isn't `main` or `master` and uses required approvers then it will stop deploying to prod. I think most likely this won't actually break for anyone though.

Looking for feedback:
* The config property is `prod_branch_names` but really there can be only a single prod branch. It takes a list though since I want to cover both `main` and `master`. Could just pick `main` or another option is to accept a `str` only too and just document a single value. A validator will convert it to a list and if not provided then it will still be `main` and `master`. Could also just leave it as is. 


Patch:
diff --git a/docs/integrations/github.md b/docs/integrations/github.md
index 18f6a41d..450e7af8 100644
--- a/docs/integrations/github.md
+++ b/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
@@ -285,18 +286,19 @@ Below is an example of how to define the default config for the bot in either YA
 
 ### Configuration Properties
 
-| Option                                | Description                                                                                                                                                                                                                                                                                                                                                                                               |  Type  | Required |
-|---------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:------:|:--------:|
-| `invalidate_environment_after_deploy` | Indicates if the PR environment created should be automatically invalidated after changes are deployed. Invalidated environments are cleaned up automatically by the Janitor. Default: `True`                                                                                                                                                                                                             |  bool  |    N     |
-| `merge_method`                        | The merge method to use when automatically merging a PR after deploying to prod. Defaults to `None` meaning automatic merge is not done. Options: `merge`, `squash`, `rebase`                                                                                                                                                                                                                             | string |    N     |
-| `enable_deploy_command`               | Indicates if the `/deploy` command should be enabled in order to allowed synchronized deploys to production. Default: `False`                                                                                                                                                                                                                                                                             |  bool  |    N     |
-| `command_namespace`                   | The namespace to use for SQLMesh commands. For example if you provide `#SQLMesh` as a value then commands will be expected in the format of `#SQLMesh/<command>`. Default: `None` meaning no namespace is used.                                                                                                                                                                                           | string |    N     |
-| `auto_categorize_changes`             | Auto categorization behavior to use for the bot. If not provided then the project-wide categorization behavior is used. See [Auto-categorize model changes](https://sqlmesh.readthedocs.io/en/stable/guides/configuration/#auto-categorize-model-changes) for details.                                                                                                                                    |  dict  |    N     |
+| Option                                | Description                                                                                                                                                                                                                                                                                                                                                                                              |  Type  | Required |
+|---------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:------:|:--------:|
+| `invalidate_environment_after_deploy` | Indicates if the PR environment created should be automatically invalidated after changes are deployed. Invalidated environments are cleaned up automatically by the Janitor. Default: `True`                                                                                                                                                                                                            |  bool  |    N     |
+| `merge_method`                        | The merge method to use when automatically merging a PR after deploying to prod. Defaults to `None` meaning automatic merge is not done. Options: `merge`, `squash`, `rebase`                                                                                                                                                                                                                            | string |    N     |
+| `enable_deploy_command`               | Indicates if the `/deploy` command should be enabled in order to allowed synchronized deploys to production. Default: `False`                                                                                                                                                                                                                                                                            |  bool  |    N     |
+| `command_namespace`                   | The namespace to use for SQLMesh commands. For example if you provide `#SQLMesh` as a value then commands will be expected in the format of `#SQLMesh/<command>`. Default: `None` meaning no namespace is used.                                                                                                                                                                                          | string |    N     |
+| `auto_categorize_changes`             | Auto categorization behavior to use for the bot. If not provided then the project-wide categorization behavior is used. See [Auto-categorize model changes](https://sqlmesh.readthedocs.io/en/stable/guides/configuration/#auto-categorize-model-changes) for details.                                                                                                                                   |  dict  |    N     |
 | `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     |	
+| `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     |
+| `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",
         )
     )
     ```
diff --git a/sqlmesh/integrations/github/cicd/command.py b/sqlmesh/integrations/github/cicd/command.py
index 2bf99675..86bb6425 100644
--- a/sqlmesh/integrations/github/cicd/command.py
+++ b/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."
diff --git a/sqlmesh/integrations/github/cicd/config.py b/sqlmesh/integrations/github/cicd/config.py
index 99f98a63..b2733293 100644
--- a/sqlmesh/integrations/github/cicd/config.py
+++ b/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 @@ class GithubCICDBotConfig(BaseConfig):
 
         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",
diff --git a/sqlmesh/integrations/github/cicd/controller.py b/sqlmesh/integrations/github/cicd/controller.py
index bf0d4a40..267e1dff 100644
--- a/sqlmesh/integrations/github/cicd/controller.py
+++ b/sqlmesh/integrations/github/cicd/controller.py
@@ -76,7 +76,7 @@ class PullRequestInfo(PydanticModel):
         return cls(
             owner=owner,
             repo=repo,
-            pr_number=pr_number,
+            pr_number=int(pr_number),
         )
 
 
@@ -444,6 +444,10 @@ class GithubController:
     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:
         """


Test patch:
diff --git a/tests/integrations/github/cicd/fixtures.py b/tests/integrations/github/cicd/fixtures.py
index 2a60378c..1ff203b7 100644
--- a/tests/integrations/github/cicd/fixtures.py
+++ b/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
 
diff --git a/tests/integrations/github/cicd/test_config.py b/tests/integrations/github/cicd/test_config.py
index 50378fc2..d42a5bdb 100644
--- a/tests/integrations/github/cicd/test_config.py
+++ b/tests/integrations/github/cicd/test_config.py
@@ -40,6 +40,7 @@ model_defaults:
     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 @@ cicd_bot:
     skip_pr_backfill: false
     pr_include_unmodified: true
     pr_environment_name: "MyOverride"
+    prod_branch_name: testing
 model_defaults:
     dialect: duckdb
 """,
@@ -85,6 +87,7 @@ model_defaults:
     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 @@ config = Config(
     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 @@ config = Config(
         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 @@ config = Config(
     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):
diff --git a/tests/integrations/github/cicd/test_integration.py b/tests/integrations/github/cicd/test_integration.py
index 6ab53303..3c5f37db 100644
--- a/tests/integrations/github/cicd/test_integration.py
+++ b/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"
+        )