Skip to content

feat: add --environment flag to sqlmesh janitor for scoped cleanup#5810

Open
mday-io wants to merge 2 commits into
SQLMesh:mainfrom
mday-io:mday-io/feature/janitor-environment-flag
Open

feat: add --environment flag to sqlmesh janitor for scoped cleanup#5810
mday-io wants to merge 2 commits into
SQLMesh:mainfrom
mday-io:mday-io/feature/janitor-environment-flag

Conversation

@mday-io
Copy link
Copy Markdown
Contributor

@mday-io mday-io commented May 22, 2026

Description

Add an --environment / -e flag to sqlmesh janitor that scopes cleanup to a single named environment, and fix sqlmesh invalidate <env> --sync to only clean up the named environment rather than all currently-expired environments.

New flag: sqlmesh janitor --environment <name> removes views, schemas, and state records for the named environment only. If the environment is not expired or does not exist, a warning is emitted. Global snapshot expiration and interval compaction are skipped when this flag is set — those operations are cross-environment and cannot be safely scoped to a single one.

Bug fix: sqlmesh invalidate <env> --sync previously called _cleanup_environments() with no filter after marking the target environment expired, which caused all other currently-expired environments to be deleted as a side effect. It now scopes the cleanup to the named environment only.

Test Plan

  • Tested manually against the sushi example project using --gateway duckdb_persistent with two environments (dev1, dev2). Invalidating dev1 and running sqlmesh janitor --environment dev1 cleaned up dev1 while leaving dev2 untouched.
  • Verified the warning message fires when targeting an environment that is not expired or does not exist.
  • Added state sync tests for name-filtered expiration and deletion (test_state_sync.py).
  • Added Context-level tests verifying invalidate_environment(..., sync=True) passes name= to _cleanup_environments and that sync=False skips cleanup entirely (test_context.py).

Checklist

  • I have run make style and fixed any issues
  • I have added tests for my changes (if applicable)
  • All existing tests pass (make fast-test)
  • My commits are signed off (git commit -s) per the DCO

@mday-io mday-io force-pushed the mday-io/feature/janitor-environment-flag branch from df7b55c to 00992be Compare May 22, 2026 21:28
@mday-io mday-io marked this pull request as draft May 22, 2026 21:34
@mday-io mday-io force-pushed the mday-io/feature/janitor-environment-flag branch from 00992be to e3ca817 Compare May 22, 2026 21:42
@mday-io mday-io marked this pull request as ready for review May 30, 2026 10:53
@StuffbyYuki StuffbyYuki force-pushed the mday-io/feature/janitor-environment-flag branch from 7ad7a4c to fa4cd32 Compare May 31, 2026 05:36
@StuffbyYuki
Copy link
Copy Markdown
Collaborator

@mday-io Not a blocker for merge, but a few things I noticed:

  • Notebook magic doesn’t expose --environment. I don' think it's too big of a deal, but it would be nice that notebook magic is aligned as well
  • get_expired_environments and delete_expired_environments have dup logic, where it might be able to be put in to _create_expiration_filter_expr instead. The dup part:
where: exp.Expr = self._create_expiration_filter_expr(current_ts)
        if name is not None:
            where = exp.and_(t.cast(exp.Condition, where), exp.column("name").eq(name))
  • Should we add a Context test that invalidate_environment(..., sync=True) calls _cleanup_environments(name=...). The state sync test covers targeted deletion, but not the Context wiring where the --sync bug lived.

Let me know if I'm missing anything!

@StuffbyYuki StuffbyYuki self-requested a review May 31, 2026 06:16
Signed-off-by: Michael Day <michael.day@cloudkitchens.com>
Signed-off-by: mday-io <mdaytn@gmail.com>
@mday-io mday-io force-pushed the mday-io/feature/janitor-environment-flag branch from fa4cd32 to 3fa6132 Compare June 1, 2026 22:16
…nvalidate 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
@mday-io mday-io force-pushed the mday-io/feature/janitor-environment-flag branch from 3fa6132 to e3e0b07 Compare June 1, 2026 22:16
@mday-io
Copy link
Copy Markdown
Contributor Author

mday-io commented Jun 1, 2026

@mday-io Not a blocker for merge, but a few things I noticed:

  • Notebook magic doesn’t expose --environment. I don' think it's too big of a deal, but it would be nice that notebook magic is aligned as well
  • get_expired_environments and delete_expired_environments have dup logic, where it might be able to be put in to _create_expiration_filter_expr instead. The dup part:
where: exp.Expr = self._create_expiration_filter_expr(current_ts)
        if name is not None:
            where = exp.and_(t.cast(exp.Condition, where), exp.column("name").eq(name))
  • Should we add a Context test that invalidate_environment(..., sync=True) calls _cleanup_environments(name=...). The state sync test covers targeted deletion, but not the Context wiring where the --sync bug lived.

Let me know if I'm missing anything!

Not tackling the notebooks wrapper for now, but created a new commit for the other comments. Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants