Skip to content

fix(config): allow --minimize-reads type-scoped mode with --cleanup - #653

Merged
riyazsh merged 1 commit into
mainfrom
riyaz.shiraguppi/minimize-reads-cleanup-coexist
Jul 30, 2026
Merged

fix(config): allow --minimize-reads type-scoped mode with --cleanup#653
riyazsh merged 1 commit into
mainfrom
riyaz.shiraguppi/minimize-reads-cleanup-coexist

Conversation

@riyazsh

@riyazsh riyazsh commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

What

--minimize-reads has two sub-modes selected in configuration.py:

  • ID-targeted (via --filter exact-ids or --id-file): state.destination[type] loads only the listed IDs.
  • Type-scoped (via --resources only): state.destination[type] loads all IDs of the requested types via the storage backend's prefix listing.

The blanket rejection at the old configuration.py:590-591 conflated the two. Cleanup's diff destination[type] - source[type] over an ID-targeted load is unsafe (would delete legitimate destination records outside the scope), but over a type-scoped load it is well-defined and functionally identical to full-load-then-cleanup for the types in scope.

Fix

Refine the guardrail to fire only on ID-targeted + cleanup. Move the check after the ID-targeting decision so it can gate on the actual scoping variable (`_state_exact_ids is not None`), not on flag presence. An `--id-file` whose types are disjoint from `--resources` correctly falls through to type-scoped and is accepted.

Tests

  • Accept: type-scoped + `--cleanup=force` — guard must not fire; invocation must progress past validation (dual assertion locks in negative + positive).
  • Accept: `--id-file` with types disjoint from `--resources` + cleanup — regression guard for the fall-through case.
  • Reject: `--id-file` matching `--resources` + cleanup — guard fires with the sub-mode-aware message.
  • Reject: exact-id `--filter` + cleanup — same guard.

Full unit suite passes locally (1082 tests, no regressions).

--minimize-reads has two sub-modes: ID-targeted (--filter exact-ids or
--id-file) and type-scoped (--resources only). Type-scoped loads
state.destination[type] fully per type via prefix listing, so the
cleanup diff destination[type] - source[type] is well-defined. ID-
targeted loads only listed IDs, and cleanup's diff over that scope
would delete legitimate destination records outside the scope.

Refine the guardrail to fire only on ID-targeted + cleanup. Guard is
placed after the ID-targeting decision and gates on the actual
scoping variable (_state_exact_ids is not None), so an --id-file
whose types are disjoint from --resources correctly falls through to
type-scoped and is accepted.
@riyazsh
riyazsh requested a review from a team as a code owner July 30, 2026 19:01
@riyazsh
riyazsh merged commit 2ae8655 into main Jul 30, 2026
12 of 13 checks passed
@riyazsh
riyazsh deleted the riyaz.shiraguppi/minimize-reads-cleanup-coexist branch July 30, 2026 19:39
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.

2 participants