Repository navigation
fix: give inline providers in Factory kwargs the scope of the providers using them - #734
Merged
Merged
Conversation
…rs using them A provider written inline in a Factory's kwargs was never stamped and always resolved at APP scope. An inline cache=True Factory under a REQUEST parent was silently shared by every request, and inline providers over request-scoped sources failed validate(). At registration, each inline provider without its own scope now takes the shallowest scope among the providers that use it, and is frozen there. Closes #732
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: f4388f7 | Previous: 53b159a | Ratio |
|---|---|---|---|
benchmarks/test_guard_by_type.py::test_g16_resolve_by_type |
6466805.306357793 iter/sec (stddev: 1.3121582456690594e-8) |
3528043.340465543 iter/sec (stddev: 1.6201518205887247e-8) |
0.55 |
benchmarks/test_guard_by_type.py::test_g17_resolve_by_type_large_registry |
6621238.788767033 iter/sec (stddev: 1.090021254611569e-8) |
3355243.408316017 iter/sec (stddev: 1.8164558621756206e-8) |
0.51 |
benchmarks/test_guard_cold.py::test_g8_cold_first_resolve |
20255.543147894958 iter/sec (stddev: 0.000034777612357906735) |
15985.466533123472 iter/sec (stddev: 0.00005295668444961533) |
0.79 |
benchmarks/test_guard_cold.py::test_g8b_cold_first_resolve_cached |
16520.339974807233 iter/sec (stddev: 0.00011371254902378454) |
12554.9954318969 iter/sec (stddev: 0.00013370621165276512) |
0.76 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[1] |
824.3026484813439 iter/sec (stddev: 0.00009644152859828223) |
473.79872455449373 iter/sec (stddev: 0.00014651882954410577) |
0.57 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[2] |
858.8399259443597 iter/sec (stddev: 0.000042017112523877864) |
467.05031933576873 iter/sec (stddev: 0.000054803573838018285) |
0.54 |
benchmarks/test_guard_concurrency.py::test_g14_concurrent_cached_hit[4] |
772.2128132263979 iter/sec (stddev: 0.00005111972203707759) |
434.21284255978355 iter/sec (stddev: 0.00007120359374410614) |
0.56 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[1] |
10954.606302438318 iter/sec (stddev: 0.000013141657363414782) |
13420.107750030218 iter/sec (stddev: 0.000011020582165937664) |
1.23 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[2] |
8936.378791660734 iter/sec (stddev: 0.000009717501673477735) |
6420.619605826631 iter/sec (stddev: 0.000022910120732793774) |
0.72 |
benchmarks/test_guard_concurrency.py::test_g15_concurrent_first_resolve[4] |
3008.0766104979375 iter/sec (stddev: 0.00007452439516799184) |
3334.6056706576665 iter/sec (stddev: 0.000041783773314964245) |
1.11 |
benchmarks/test_guard_concurrency.py::test_g15b_concurrent_first_resolve_sibling_children[1] |
16103.314572004729 iter/sec (stddev: 0.000016837219885167984) |
10104.649646081618 iter/sec (stddev: 0.000010103075952638162) |
0.63 |
benchmarks/test_guard_concurrency.py::test_g15b_concurrent_first_resolve_sibling_children[2] |
5944.2441784550665 iter/sec (stddev: 0.000016692793046936357) |
4206.51536650179 iter/sec (stddev: 0.00003646404635374897) |
0.71 |
benchmarks/test_guard_concurrency.py::test_g15b_concurrent_first_resolve_sibling_children[4] |
2097.003869024552 iter/sec (stddev: 0.00007946649561301732) |
2004.621487785671 iter/sec (stddev: 0.00004199813555517138) |
0.96 |
benchmarks/test_guard_concurrency.py::test_g15c_worker_pool_floor_control[1] |
18828.94624121577 iter/sec (stddev: 0.0000030447117817926802) |
40407.71383135906 iter/sec (stddev: 0.000005008385511484375) |
2.15 |
benchmarks/test_guard_concurrency.py::test_g15c_worker_pool_floor_control[2] |
13462.072629582195 iter/sec (stddev: 0.000010368811347900758) |
13369.118005729773 iter/sec (stddev: 0.00001488410635264421) |
0.99 |
benchmarks/test_guard_concurrency.py::test_g15c_worker_pool_floor_control[4] |
5528.090924932034 iter/sec (stddev: 0.00004917509772077332) |
5553.574780567612 iter/sec (stddev: 0.000035051702317751075) |
1.00 |
benchmarks/test_guard_lifecycle.py::test_g6_build_child_container |
2565427.212579573 iter/sec (stddev: 3.529915917772928e-8) |
1598547.7513419166 iter/sec (stddev: 3.169389972863768e-8) |
0.62 |
benchmarks/test_guard_lifecycle.py::test_g6b_build_child_container_auto_scope |
2627585.3633522335 iter/sec (stddev: 3.5873285997138694e-8) |
1693646.884953151 iter/sec (stddev: 4.284472773928401e-8) |
0.64 |
benchmarks/test_guard_lifecycle.py::test_g7_request_lifecycle_batch |
4834.347210597371 iter/sec (stddev: 0.000010404173307677788) |
3227.607832274615 iter/sec (stddev: 0.000010446514091967131) |
0.67 |
benchmarks/test_guard_lifecycle.py::test_g7c_event_loop_floor_control |
93912.97302121205 iter/sec (stddev: 0.0000011139490110642276) |
58042.984227328125 iter/sec (stddev: 0.0000018588750309430483) |
0.62 |
benchmarks/test_guard_lifecycle.py::test_g7b_request_cycle_sync |
705034.329353943 iter/sec (stddev: 7.499719381263048e-8) |
423107.39250427077 iter/sec (stddev: 7.008524311733466e-8) |
0.60 |
benchmarks/test_guard_lifecycle.py::test_g13_teardown_at_scale |
100305.79034639939 iter/sec (stddev: 0.0000010297399522293532) |
63280.93450529563 iter/sec (stddev: 0.000002147406152624959) |
0.63 |
benchmarks/test_guard_lifecycle.py::test_g13b_teardown_at_scale_async_no_finalizers |
914.1726767279387 iter/sec (stddev: 0.0007995056740938146) |
608.1669957544557 iter/sec (stddev: 0.0007670680174930931) |
0.67 |
benchmarks/test_guard_resolve.py::test_g1_transient_resolve |
3848836.5592788975 iter/sec (stddev: 2.9218306707786818e-8) |
2203238.8713104604 iter/sec (stddev: 2.541051829664956e-8) |
0.57 |
benchmarks/test_guard_resolve.py::test_g2_cached_resolve |
6607718.767052108 iter/sec (stddev: 1.2853720116799686e-8) |
3590759.037988996 iter/sec (stddev: 9.830744313484482e-9) |
0.54 |
benchmarks/test_guard_resolve.py::test_g3_deep_chain |
1349872.7390613668 iter/sec (stddev: 5.503561549712403e-8) |
766487.7307444098 iter/sec (stddev: 1.9240825405249651e-7) |
0.57 |
benchmarks/test_guard_resolve.py::test_g4_wide_resolve |
803202.6581528357 iter/sec (stddev: 3.1577165671321265e-7) |
467857.90463009063 iter/sec (stddev: 4.068391284538452e-7) |
0.58 |
benchmarks/test_guard_resolve.py::test_g5_cross_scope |
3365638.980029384 iter/sec (stddev: 2.0231344550955927e-8) |
1989677.2559338638 iter/sec (stddev: 2.091452994252324e-8) |
0.59 |
benchmarks/test_guard_resolve.py::test_g9_context_resolve |
2472857.6058105812 iter/sec (stddev: 1.0744468996592735e-7) |
1393524.4868091359 iter/sec (stddev: 1.3368616477698553e-7) |
0.56 |
benchmarks/test_guard_resolve.py::test_g12_override_active_resolve |
1393061.9801836472 iter/sec (stddev: 2.9543775313354946e-8) |
829711.1945406267 iter/sec (stddev: 7.339309737771445e-8) |
0.60 |
benchmarks/test_guard_resolve.py::test_g18_alias_hop |
6507185.103635749 iter/sec (stddev: 1.317585236791968e-8) |
3431059.1621292867 iter/sec (stddev: 2.0227134209957854e-8) |
0.53 |
benchmarks/test_guard_validate.py::test_g10_validate_deep_chain |
51211.62260735551 iter/sec (stddev: 0.000001726536954864777) |
30402.232480488412 iter/sec (stddev: 0.00023546839534572183) |
0.59 |
benchmarks/test_guard_validate.py::test_g11_validate_wide |
31855.057280669324 iter/sec (stddev: 0.0000020868955471951034) |
20553.440998470625 iter/sec (stddev: 0.000024398345850373547) |
0.65 |
This comment was automatically generated by workflow using github-action-benchmark.
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.
Closes #732.
A provider written inline in a Factory's
kwargsis not a class attribute, so no group stamped it and it always resolved at APP scope, whatever the scope of the provider using it. An explicitscope=on the parent did not reach it either.When the container registers providers, each inline provider without its own
scope=now takes the shallowest scope among the providers that use it, and that scope is frozen at that point.Behaviour change
Factory(..., cache=True)under a REQUEST parent used to be created once and shared by every request. It is now one instance per request. That sharing was the bug, but code that relied on it changes without an error. To keep an app-wide instance, setscope=Scope.APPon the inline provider.Factory(lambda r: r.user, kwargs={"r": request})in a REQUEST group, used to failvalidate()withInvalidScopeDependencyError. It now validates and resolves.ContextProvidernow reads the context of the parent's scope. Before, it looked in the APP container and raisedContextValueNotSetError.scope=, used inline, now takes its parents' scope. A library that means APP has to writescope=Scope.APP..scopestill reads APP.Design
ProvidersRegistry._add, under the lock and after the duplicate-type check. It coversContainer(groups=...)andcontainer.add_providers(), and the resolve path does not change.kwargswith_declared_as is Nonethat is not itself in the batch. A class attribute of another group keeps its own scope wherever it is referenced.GroupScopeConflictError.scopenow checks, in order: explicitscope=, group default, inline scope,Scope.APP. Providers with_takes_group_scope = False(Alias, ContainerProvider) are never stamped.ProviderScopeFrozenError, the same as for class attributes.Differences from the issue
ProviderScopeFrozenErrorwhen a lateradd_providers()adds a shallower parent. Here the first registration fixes the scope, and the new parent failsvalidate()withInvalidScopeDependencyError. The existing check reports it, so no new exception class or troubleshooting page is needed.provider.derive, merged as feat: add provider.derive(fn) to inject a value computed from another provider #733) landed first, so this PR adds the walk through_Derived. A derived provider has_takes_group_scope = Falseand is never stamped itself, but it passes its parents' scope down. Inkwargs={"session": providers.Factory(DbSession, cache=True).derive(fn)}, the inlineDbSessionfactory takes the scope of the provider that uses the derived one.Tests and docs
tests/test_inline_scope.pycovers the repro cases from the issue, the shared-parent pattern, nested inline providers, explicit inline scope, class attributes of another group, the freeze at first registration and the group re-stamp error, and an inline source behind an inlinederive.docs/providers/scopes.mdhas a new "Inline providers" section and an extra step in the priority list.docs/providers/factories.mdlinks to it, andCONTEXT.mdgains an "Inline provider" entry.