Skip to content

fix(sorter): skip transitive dep-only nodes not in --resources#610

Open
riyazsh wants to merge 1 commit into
mainfrom
riyaz.shiraguppi/skip-transitive-deps-in-sorter
Open

fix(sorter): skip transitive dep-only nodes not in --resources#610
riyazsh wants to merge 1 commit into
mainfrom
riyaz.shiraguppi/skip-transitive-deps-in-sorter

Conversation

@riyazsh

@riyazsh riyazsh commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Under --minimize-reads, when a sync run is scoped via --resources, _resource_connections lazy-loads referenced dependencies via ensure_resource_loaded and adds them to the dependency graph as edges for ordering. Those dep-only nodes were being yielded by TopologicalSorter and dispatched to _apply_resource_cb, which then ran connect_resources() on them and looked their OWN sub-deps up in state.destination — but ensure_resource_loaded is 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:

  1. state._minimize_reads is true (full-load runs are unaffected),
  2. node[0] is not in resources_arg (operator did not request this type), and
  3. node[1] is already present in state.destination[node[0]] (the parent's ensure_resource_loaded already populated the mapping the parent's connect_id() needs; nothing more to do).

If any condition fails the node is dispatched as before — in particular, --force-missing-dependencies still 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_arg case, a mixed batch (skippable + dispatchable in the same get_ready()), the asyncio.sleep(0) yield between batches (patched-assert), the --force-missing-dependencies flow (destination absent → still dispatched), and full-load mode (_minimize_reads=False → nothing skipped).

@riyazsh riyazsh requested a review from a team as a code owner July 6, 2026 18:22
@riyazsh riyazsh force-pushed the riyaz.shiraguppi/skip-transitive-deps-in-sorter branch from f7aaf1a to ecb99ed Compare July 6, 2026 18:42
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.
@riyazsh riyazsh force-pushed the riyaz.shiraguppi/skip-transitive-deps-in-sorter branch from ecb99ed to 8e7397f Compare July 6, 2026 19:00
@riyazsh

riyazsh commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

The test-integrations job failure is unrelated to this PR. The 8 failing tests are exactly the set #611 identifies as pre-existing test-org fixture drift (missing S3 bucket for logs_archives, empty metric_tag_configurations). Same 8 tests have been failing on every scheduled main run since 2026-07-02 per #611's evidence. All other CI checks pass: 4x test (matrix), 4x Build and test executables, changelog, devflow/mergegate.

Suggest merging #611 first, then rebasing this on top to make CI fully green.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant