Skip to content

Commit f082aea

Browse files
committed
Gate the existence check behind must_exist for user-facing callers
The existence check was applied unconditionally in GenericContext.invalidate_environment, which broke GithubController.try_invalidate_pr_environment. That method invalidates the PR environment after a prod deploy, and the environment may never have been created — a forward-only deploy being one case — so it relies on a missing environment being a silent no-op. Raising there turned a routine cleanup into a failed deploy, which is what test_deploy_prod_forward_only caught. Move the check behind must_exist=False and have the two user-facing entry points, the invalidate CLI command and the %invalidate magic, opt in. The reported behavior is unchanged; library callers keep the lenient path. Adds a regression test pinning the no-op default. Signed-off-by: Nishchay Mahor <nishchaymahor@gmail.com>
1 parent 23bc02e commit f082aea

4 files changed

Lines changed: 32 additions & 5 deletions

File tree

sqlmesh/cli/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ def run(ctx: click.Context, environment: t.Optional[str] = None, **kwargs: t.Any
641641
def invalidate(ctx: click.Context, environment: str, **kwargs: t.Any) -> None:
642642
"""Invalidate the target environment, forcing its removal during the next run of the janitor process."""
643643
context = ctx.obj
644-
context.invalidate_environment(environment, **kwargs)
644+
context.invalidate_environment(environment, must_exist=True, **kwargs)
645645

646646

647647
@cli.command("janitor")

sqlmesh/core/context.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1870,16 +1870,23 @@ def apply(
18701870
)
18711871

18721872
@python_api_analytics
1873-
def invalidate_environment(self, name: str, sync: bool = False) -> None:
1873+
def invalidate_environment(
1874+
self, name: str, sync: bool = False, must_exist: bool = False
1875+
) -> None:
18741876
"""Invalidates the target environment by setting its expiration timestamp to now.
18751877
18761878
Args:
18771879
name: The name of the environment to invalidate.
18781880
sync: If True, the call blocks until the environment is deleted. Otherwise, the environment will
18791881
be deleted asynchronously by the janitor process.
1882+
must_exist: If True, raise if the environment doesn't exist instead of silently doing nothing.
1883+
Used by the user-facing entry points, where a mistyped name should be reported rather than
1884+
look like it succeeded. Internal callers such as
1885+
`GithubController.try_invalidate_pr_environment` rely on the default no-op behavior, since
1886+
a PR environment may never have been created.
18801887
"""
18811888
name = Environment.sanitize_name(name)
1882-
if self.state_sync.get_environment(name) is None:
1889+
if must_exist and self.state_sync.get_environment(name) is None:
18831890
raise SQLMeshError(f"Environment '{name}' was not found.")
18841891
self.state_sync.invalidate_environment(name)
18851892
if sync:

sqlmesh/magics.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -983,7 +983,7 @@ def diff(self, context: Context, line: str) -> None:
983983
def invalidate(self, context: Context, line: str) -> None:
984984
"""Invalidate the target environment, forcing its removal during the next run of the janitor process."""
985985
args = parse_argstring(self.invalidate, line)
986-
context.invalidate_environment(args.environment)
986+
context.invalidate_environment(args.environment, must_exist=True)
987987

988988
@magic_arguments()
989989
@argument(

tests/core/test_context.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1937,11 +1937,31 @@ def test_invalidate_environment_nonexistent_raises(sushi_context, mocker: Mocker
19371937
state_sync_mock.get_environment.return_value = None
19381938

19391939
with pytest.raises(SQLMeshError, match="Environment 'doesnotexist' was not found"):
1940-
sushi_context.invalidate_environment("doesnotexist")
1940+
sushi_context.invalidate_environment("doesnotexist", must_exist=True)
19411941

19421942
state_sync_mock.invalidate_environment.assert_not_called()
19431943

19441944

1945+
def test_invalidate_environment_nonexistent_is_a_noop_by_default(
1946+
sushi_context, mocker: MockerFixture
1947+
) -> None:
1948+
"""Without must_exist, invalidating a missing environment stays a no-op.
1949+
1950+
Internal callers depend on this. `GithubController.try_invalidate_pr_environment`
1951+
invalidates the PR environment after a prod deploy, and that environment may never
1952+
have been created — a forward-only deploy, for instance. Raising there turns a
1953+
routine cleanup into a failed deploy.
1954+
"""
1955+
state_sync_mock = mocker.patch.object(
1956+
type(sushi_context), "state_sync", new_callable=mocker.PropertyMock
1957+
).return_value
1958+
state_sync_mock.get_environment.return_value = None
1959+
1960+
sushi_context.invalidate_environment("doesnotexist")
1961+
1962+
state_sync_mock.invalidate_environment.assert_called_once_with("doesnotexist")
1963+
1964+
19451965
@pytest.mark.slow
19461966
def test_plan_default_end(sushi_context_pre_scheduling: Context):
19471967
prod_plan_builder = sushi_context_pre_scheduling.plan_builder("prod")

0 commit comments

Comments
 (0)