fix: error when invalidating a nonexistent environment - #5878
fix: error when invalidating a nonexistent environment#5878NishchayMahor wants to merge 3 commits into
Conversation
|
good call, done. switched it to |
|
@gjesse Following up on another review since the author made the changes |
|
Friendly ping — this has @gjesse's approval and the wording change he asked for is in. Anything else needed from me, or is it just waiting on a merge? |
|
Oh I didn't notice he approved it. I just kicked off the ci tests and once they're good I'll merge it |
sqlmesh invalidate ENVIRONMENT reported success even when the environment did not exist, so a mistyped name looked like it worked. Check the environment exists first and raise a clear error (nonzero exit) otherwise. Fixes SQLMesh#5621 Signed-off-by: Nishchay Mahor <nishchaymahor@gmail.com>
Signed-off-by: Nishchay Mahor <nishchaymahor@gmail.com>
6289aec to
23bc02e
Compare
|
Thanks for kicking off CI @StuffbyYuki — sorry, that red run was my branch being stale rather than anything in the change. Rebased onto latest What happened: the only failure was a single mypy error, and it wasn't in a file this PR touches — That was fixed on Verified locally after rebasing: One thing I could not verify locally: the Note that |
|
Correcting myself — I got the last comment wrong twice, sorry. The remaining failure was mine, not stale-branch fallout. The cause: I put the existence check unconditionally in Fixed in Verified locally: Second correction: I said main's CI had been red since 2026-08-06 and implied it might explain my failure. That was misleading. The runs do show red, but the failing jobs there are I'd earlier said the |
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>
fac00c8 to
f082aea
Compare
|
That last red check was a missing DCO sign-off on my |
|
Heads up that the push reset the workflow approval — both runs are sitting at |
What
sqlmesh invalidate ENVIRONMENTreports success even when the environment doesn't exist:That's misleading — a mistyped environment name looks like it worked. This checks that the environment exists first and raises a clear error with a nonzero exit code otherwise:
How
Context.invalidate_environmentnow looks the environment up viastate_sync.get_environmentbefore invalidating, and raisesSQLMeshErrorwhen it's absent (the CLI's@error_handlermaps that to exit code 1). The message and exception match the existing "environment not found" patterns already used elsewhere incontext.py.Testing
test_invalidate_environment_nonexistent_raises(fails without the fix).test_invalidate_environment_{sync,no_sync}and thetest_invalidating_environmentintegration test still pass.sqlmesh init duckdbproject: nonexistent env →Error: Environment '...' does not exist.with exit code 1.Fixes #5621