fix(sorter): skip transitive dep-only nodes not in --resources#610
Open
riyazsh wants to merge 1 commit into
Open
fix(sorter): skip transitive dep-only nodes not in --resources#610riyazsh wants to merge 1 commit into
riyazsh wants to merge 1 commit into
Conversation
f7aaf1a to
ecb99ed
Compare
Nodes added to the graph as lazy-loaded deps of a top-level resource were being dispatched to connect_resources, producing false "missing connections" errors for their own sub-deps.
ecb99ed to
8e7397f
Compare
Contributor
Author
|
The Suggest merging #611 first, then rebasing this on top to make CI fully green. |
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.
What does this PR do?
Under
--minimize-reads, when a sync run is scoped via--resources,_resource_connectionslazy-loads referenced dependencies viaensure_resource_loadedand adds them to the dependency graph as edges for ordering. Those dep-only nodes were being yielded byTopologicalSorterand dispatched to_apply_resource_cb, which then ranconnect_resources()on them and looked their OWN sub-deps up instate.destination— butensure_resource_loadedis not recursive, so those sub-deps were never loaded, and the dispatch emitted false "missing connections" errors even when the referenced destination JSONs existed on disk.Description of the Change
run_sorter()now skips a node when ALL of these hold:state._minimize_readsis true (full-load runs are unaffected),node[0]is not inresources_arg(operator did not request this type), andnode[1]is already present instate.destination[node[0]](the parent'sensure_resource_loadedalready populated the mapping the parent'sconnect_id()needs; nothing more to do).If any condition fails the node is dispatched as before — in particular,
--force-missing-dependenciesstill creates deps that are absent from destination.Test Plan
Regression tests cover: the skip (all three conditions true), dispatch when the type IS in
resources_arg, the pre-existing missing-from-source skip, the full-resources_argcase, a mixed batch (skippable + dispatchable in the sameget_ready()), theasyncio.sleep(0)yield between batches (patched-assert), the--force-missing-dependenciesflow (destination absent → still dispatched), and full-load mode (_minimize_reads=False→ nothing skipped).