Repository navigation
fix: namespace the context keys and read the request container as a local - #76
Merged
Merged
Conversation
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.
The plugin stored its two containers in FastStream's
ContextRepounder generic names, so a user's own context entries could replace them.Changes
modern_di_root_containerandmodern_di_request_container(previouslydi_containerandrequest_container). Both are private constants;fetch_di_containerremains the public way to reach the root container, so this is not a breaking change.FromDIresolution reads the request container withContextRepo.get_local.ContextRepo.getchecks globals before locals, so a global with the same key would win over the per-message container.get_localonly sees the value the middleware set withcontext.scope.Where the request container collision shows up
On FastStream 0.7.7 a broker attached to an app gets a
ContextRepoComposition. The middleware'scontext.scopewrites to the composition's inner repo, andgetwalks the repos in order. A global passed viaFastStream(context=ContextRepo({...}))lives in a later repo, so it does not shadow the request container. A global set throughbroker.context.set_global(...)(or thecontextinjected into a handler or hook) lands in the inner repo next to the local and does shadow it. The new test uses that path.get_localalone fixes this case; the rename covers it as well.Tests
test_from_di_ignores_a_user_global_named_request_container: sets arequest_containerglobal on the broker's context and checks thatFromDIstill resolves from the request container.test_fetch_di_container_ignores_a_user_global_named_di_container: sets adi_containerglobal aftersetup_diand checks thatfetch_di_containerstill returns the container passed tosetup_di.Both failed on
mainbefore the fix. Existing tests pass unchanged; one docstring now namesget_localin place ofget.Closes #74
Closes #75