Skip to content

Commit 3fa6132

Browse files
mday-ioclaude
andcommitted
refactor: deduplicate expiration filter logic and add Context-level invalidate test
- Extend _create_expiration_filter_expr to accept optional name= param, consolidating the repeated AND-name filter from get_expired_environments and delete_expired_environments into a single place - Add test_invalidate_environment_sync_calls_cleanup_with_name: verifies that invalidate_environment(..., sync=True) passes name= to _cleanup_environments, guarding the --sync bug fix at the Context level - Add test_invalidate_environment_no_sync_skips_cleanup: verifies that sync=False does not trigger delete_expired_environments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent bbccc93 commit 3fa6132

2 files changed

Lines changed: 46 additions & 10 deletions

File tree

sqlmesh/core/state_sync/db/environment.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,9 @@ def get_expired_environments(
181181
Returns:
182182
The list of environment summaries to remove.
183183
"""
184-
where: exp.Expr = self._create_expiration_filter_expr(current_ts)
185-
if name is not None:
186-
where = exp.and_(t.cast(exp.Condition, where), exp.column("name").eq(name))
187-
return self._fetch_environment_summaries(where=where)
184+
return self._fetch_environment_summaries(
185+
where=self._create_expiration_filter_expr(current_ts, name=name)
186+
)
188187

189188
def delete_expired_environments(
190189
self, current_ts: t.Optional[int] = None, name: t.Optional[str] = None
@@ -201,12 +200,9 @@ def delete_expired_environments(
201200
current_ts = current_ts or now_timestamp()
202201
expired_environments = self.get_expired_environments(current_ts=current_ts, name=name)
203202

204-
where: exp.Expr = self._create_expiration_filter_expr(current_ts)
205-
if name is not None:
206-
where = exp.and_(t.cast(exp.Condition, where), exp.column("name").eq(name))
207203
self.engine_adapter.delete_from(
208204
self.environments_table,
209-
where=where,
205+
where=self._create_expiration_filter_expr(current_ts, name=name),
210206
)
211207

212208
# Delete the expired environments' corresponding environment statements
@@ -325,16 +321,22 @@ def _environments_query(
325321
return query.lock(copy=False)
326322
return query
327323

328-
def _create_expiration_filter_expr(self, current_ts: int) -> exp.Expr:
324+
def _create_expiration_filter_expr(
325+
self, current_ts: int, name: t.Optional[str] = None
326+
) -> exp.Expr:
329327
"""Creates a SQLGlot filter expression to find expired environments.
330328
331329
Args:
332330
current_ts: The current timestamp.
331+
name: If provided, adds an equality filter on the environment name.
333332
"""
334-
return exp.LTE(
333+
where: exp.Expr = exp.LTE(
335334
this=exp.column("expiration_ts"),
336335
expression=exp.Literal.number(current_ts),
337336
)
337+
if name is not None:
338+
where = exp.and_(t.cast(exp.Condition, where), exp.column("name").eq(name))
339+
return where
338340

339341
def _fetch_environment_summaries(
340342
self, where: t.Optional[str | exp.Expr] = None

tests/core/test_context.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,6 +1170,40 @@ def test_janitor_environment_not_expired_warning(sushi_context, mocker: MockerFi
11701170
assert "nonexistent_env" in warning_mock.call_args[0][0]
11711171

11721172

1173+
@pytest.mark.slow
1174+
def test_invalidate_environment_sync_calls_cleanup_with_name(
1175+
sushi_context, mocker: MockerFixture
1176+
) -> None:
1177+
"""invalidate_environment(..., sync=True) must pass name= to _cleanup_environments so only the
1178+
target environment is deleted, not all expired environments."""
1179+
state_sync_mock = mocker.patch.object(
1180+
type(sushi_context), "state_sync", new_callable=mocker.PropertyMock
1181+
).return_value
1182+
state_sync_mock.get_expired_environments.return_value = []
1183+
1184+
sushi_context.invalidate_environment("dev", sync=True)
1185+
1186+
state_sync_mock.invalidate_environment.assert_called_once_with("dev")
1187+
state_sync_mock.delete_expired_environments.assert_called_once()
1188+
_, kwargs = state_sync_mock.delete_expired_environments.call_args
1189+
assert kwargs.get("name") == "dev"
1190+
1191+
1192+
@pytest.mark.slow
1193+
def test_invalidate_environment_no_sync_skips_cleanup(
1194+
sushi_context, mocker: MockerFixture
1195+
) -> None:
1196+
"""invalidate_environment(..., sync=False) should not trigger _cleanup_environments at all."""
1197+
state_sync_mock = mocker.patch.object(
1198+
type(sushi_context), "state_sync", new_callable=mocker.PropertyMock
1199+
).return_value
1200+
1201+
sushi_context.invalidate_environment("dev", sync=False)
1202+
1203+
state_sync_mock.invalidate_environment.assert_called_once_with("dev")
1204+
state_sync_mock.delete_expired_environments.assert_not_called()
1205+
1206+
11731207
@pytest.mark.slow
11741208
def test_plan_default_end(sushi_context_pre_scheduling: Context):
11751209
prod_plan_builder = sushi_context_pre_scheduling.plan_builder("prod")

0 commit comments

Comments
 (0)