Conversation
added 2 commits
May 26, 2025 10:23
There was a problem hiding this comment.
Pull Request Overview
This PR adds a threadsafe flag to all instance‐retrieval methods, introduces a shared get_lock helper for locking, updates FastAPI integration to expose a InjectThreadSafe alias, and adjusts tests to expect NotImplemented as the missing‐instance sentinel.
- Introduce
threadsafeparameter onget_instance,aget_instance, and their lazy variants - Centralize locking via
injection._core.common.threading.get_lockand replace direct RLock/nullcontext usages - Extend
FastAPIInjectwiththreadsafe=Truealias (InjectThreadSafe) and update stubs/tests for new default behavior
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_singleton.py | update default None → NotImplemented assertions |
| tests/test_injectable.py | same update for injectable tests |
| tests/core/test_module.py | sync/async instance and lazy tests now expect NotImplemented |
| injection/ext/fastapi.pyi | add InjectThreadSafe type alias |
| injection/ext/fastapi.py | support threadsafe in FastAPIInject, expose InjectThreadSafe |
| injection/_core/common/threading.py | new get_lock(threadsafe) helper |
| injection/_core/scope.py | replace inline RLock/nullcontext with get_lock |
| injection/_core/module.py | add threadsafe args to find/get/aget/aget_lazy/get_lazy |
| injection/_core/common/lazy.py | remove unused alazy helper |
| injection/_core/common/asynchronous.py | minor formatting import change |
| injection/init.pyi | update stubs for threadsafe parameter and default return type |
Comments suppressed due to low confidence (3)
tests/core/test_module.py:92
- Add tests that explicitly pass
threadsafe=True(andFalse) to each getter (e.g.,aget_instance,get_instance, lazy variants) to verify that concurrent access is correctly locked or unlocked.
async def test_aget_instance_with_no_injectable_return_not_implemented(
injection/_core/common/threading.py:1
- [nitpick] Consider renaming this module (e.g.,
lock_utils.pyorsynchronization.py) to avoid confusion with Python's standardthreadingmodule.
from contextlib import nullcontext
injection/ext/fastapi.pyi:7
- Add an
__all__export in this stub to includeInjectThreadSafeso users of the stub file see the alias in auto‐completion and documentation.
type InjectThreadSafe[T, *Metadata] = Inject[T, *Metadata]
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.
No description provided.