refactor(providers): single scope-source field; explicit group-stamp opt-out - #425
Open
lesnik512 wants to merge 1 commit into
Open
refactor(providers): single scope-source field; explicit group-stamp opt-out#425lesnik512 wants to merge 1 commit into
lesnik512 wants to merge 1 commit into
Conversation
…opt-out Replace _scope_defaulted + _stamping_group with one _scope_source field, and give Alias and the container provider a declarative _takes_group_scope = False instead of relying on a placeholder explicit scope to dodge stamping.
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.
Why
AbstractProvidercarried four fields to answer one question — what scope, and may agroup still change it?
scope_scope_defaulted_stamping_group_registered_scope_defaultedand_stamping_groupare two encodings of the same fact: where thecurrent scope came from.
_scope_defaultedexists only because the constructor collapsesUNSET → Scope.APPon the first line, erasing why a provider is APP-scoped, which isexactly what the precedence rule needs (explicit
scope=> group default > APP).Separately,
Aliasand_ContainerProvideropted out of group stamping by passing aconcrete
scope=Scope.APPin order to leave_scope_defaultedFalse. That is a sideeffect standing in for an intent, and
alias.pyneeded two comment lines to explain thetrick.
Design
One provenance field,
_scope_source, whose three states are the three ways a scope ischosen:
None— nobody chose;scopeis theScope.APPfallback and a group may stamp itstr— that group's default stamped it (the name is whatGroupScopeConflictErrorreports)_EXPLICIT_SCOPE— the constructor'sscope=argument; a group default is a silent no-op_stamp_group_scopebecomes a flat read of that one field instead of two booleans thathad to stay in sync:
_takes_group_scope: ClassVar[bool]is the second half:Aliasand_ContainerProviderset it False and say so where it is true of them.
Aliasnow passesscope=types.UNSET,which is honest — an alias chooses no scope, its effective scope derives from its source —
where the old
Scope.APPwas a placeholder doing double duty as the opt-out.Net: one slot fewer, one predicate per branch, and the un-stampability of a derived-scope
provider is declared rather than arranged.
Behaviour is unchanged. Existing tests, including every case in the group-scope precedence
and freeze suites, pass untouched.
Non-goals
delete
_registered,ProviderScopeFrozenError, and the freeze suite outright, butGroup.svc.scopereflecting the group default before any container exists is documented(
docs/providers/scopes.md) and asserted, andprovider.scopeis read insuggester.py,dependency_graph.py,integrations.py, andexceptions.pywhere no registry is in hand.every user-facing statement in
docs/providers/scopes.mdstill holds verbatim._takes_group_scopestays private. It is not a provider-extension seam.Verification
just test-ci— 511 passed, 100.00% line coverage (the gate).just lint-ci— ruff + ty + planning bundle checks all clean.tests/test_group.py, both written and confirmed passingbefore the refactor so they are real characterization, not tests fitted to the new code:
test_group_scope_alias_still_resolves_from_the_source_container— anAliasin aREQUEST group over an APP source still resolves from the APP container. The pre-existing
test_group_scope_alias_keeps_derived_scopeonly asserted the stored attribute; thispins the consequence.
test_group_scope_does_not_stamp_the_container_provider—container_providerispublic, so a group body may list it; a group default must not move the one shared
singleton.