Fix: allow --select-model to plan a model deletion#5759
Open
viniciusnunest wants to merge 1 commit intoSQLMesh:mainfrom
Open
Fix: allow --select-model to plan a model deletion#5759viniciusnunest wants to merge 1 commit intoSQLMesh:mainfrom
viniciusnunest wants to merge 1 commit intoSQLMesh:mainfrom
Conversation
When a model file is deleted locally but still exists in the deployed environment, --select-model now correctly plans the removal instead of raising PlanError. Fixes SQLMesh#5741 Signed-off-by: vinicius <viniciusnunes.tavares@gmail.com>
93e5928 to
da9a69c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #5741
Right now if you delete a model file locally and try to plan its removal with
--select-model=<deleted-model>, you get:The problem is that
expand_model_selectionsonly looks at local models. Sincethe file is gone, it finds nothing,
backfill_modelsends up empty, and_plan()raises. The workaround was adding some unrelated active model to theselection, which works but it is not great.
The thing is, the underlying machinery already handles deletions fine. When
backfill_modelshas at least one entry, deleted models show up correctly incontext_diff.removed_snapshots. So the fix is about making the selector andplan validation aware that "no local matches" can still be valid when matching
models pending deletion in the deployed environment.
Changes:
_load_env_models()fromselect_models()to avoid duplicatingthe environment loading logic
expand_model_selections_with_env()that expands selections againstboth local and environment models
_plan(), whenbackfill_modelsis empty, it now checks whether theselection matched models in the environment that were deleted locally. If so,
PlanErroris suppressed and the deletions surface viaremoved_snapshotsraises
PlanErroras beforeTest Plan
test_expand_model_selections_with_env: deleted models found in env, localmodels still work, wildcards match, non-existent returns empty
test_expand_model_selections_with_env_fallback: fallback environment is usedwhen target env is missing
test_expand_model_selections_with_env_expired: expired environments ignoredtest_plan_select_model_deleted_model: end-to-end with the sushi example,deploys to prod, deletes model file, reloads, runs plan and checks that the
model appears in
removed_snapshotstest_plan_select_model_deleted_model_still_rejects_nonexistent: negativecase, makes sure garbage input still fails
Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCO