Repository navigation
Fix: load materializations before run janitor - #5723
cnahed-kpler wants to merge 1 commit into
Conversation
|
@cnahed-kpler could you take a look at resolving conflicts? |
|
I just kicked off the workflows. Once they all pass, I'll merge! |
|
@cnahed-kpler Can you add DCO checks? |
|
You can ignore those failed style ci tests, there was a bug from another merge, but the fix got merged. |
|
@cnahed-kpler Just following up |
|
This PR should close this issue #5761 (comment) |
|
@cnahed-kpler Bump |
|
Reproduced on v0.236.2. sqlmesh janitor fails with "Materialization strategy ... not found" because materializations/ is never imported when the CLI builds the context with load=False. Current workaround: import the custom materialization classes in config.py, would remove our workaround once merged |
|
Since the PR owner @cnahed-kpler isn’t responding or active, and this change is simple but essential, I can’t resolve the conflicts or prepare it for merge because I don’t have permission to push commits to this branch. @StuffbyYuki Should I open a new PR with the same change and have this one closed? |
|
@lafirm that's fine with me. Let me know once you open your PR, I'll close this one |
|
@StuffbyYuki Created a new PR #6162, so this can be closed now. |
This PR makes sure materializations are loaded before cleanup-related logic runs.
That matters because janitor/cleanup operations depend on materialization definitions already being available.
This occurs here: https://github.com/TobikoData/sqlmesh/blob/6e69ce6cfbceab8e7c5f43cdaa7ac7a5d9015cd6/sqlmesh/core/snapshot/evaluator.py#L2877