feat(async): greenlet Submitter and PluginManager.run_async - #11
Open
RonnyPfannschmidt wants to merge 3 commits into
Open
feat(async): greenlet Submitter and PluginManager.run_async#11RonnyPfannschmidt wants to merge 3 commits into
RonnyPfannschmidt wants to merge 3 commits into
Conversation
Reviewer's GuideIntroduces a greenlet-based async Submitter and wires it through PluginManager and hook callers so that awaitable hook results can be optionally awaited via the new PluginManager.run_async API, along with tests, benchmarks, and packaging support for the async extra. Sequence diagram for PluginManager.run_async awaiting hook resultssequenceDiagram
actor User
participant PluginManager
participant NormalHookCaller
participant _multicall
participant HookImpl
participant Submitter
User->>PluginManager: run_async(lambda: hook.my_hook())
PluginManager->>Submitter: run(sync_func)
activate Submitter
Submitter->>NormalHookCaller: sync_func()
NormalHookCaller->>_multicall: _hookexec(..., async_submitter)
_multicall->>HookImpl: function(*args)
HookImpl-->>_multicall: awaitable_result
_multicall->>Submitter: maybe_submit(awaitable_result)
Submitter-->>_multicall: awaited_result
_multicall-->>NormalHookCaller: results
NormalHookCaller-->>Submitter: results
Submitter-->>PluginManager: final_result
PluginManager-->>User: awaited final_result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This was referenced Jul 24, 2026
refactor(caller): Protocol HookCaller, split callers, CompletionHook multicall
pytest-dev/pluggy#708
Draft
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/pluggy/_manager.py" line_range="139" />
<code_context>
wrapper_impls: Sequence[WrapperImpl],
caller_kwargs: Mapping[str, object],
firstresult: bool,
+ async_submitter: Submitter,
) -> object | list[object]:
"""Execute a call into multiple python functions/methods and return the
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing the async_submitter parameter from the public/internal hookexec functions and instead using the manager-owned submitter inside PluginManager to keep the async wiring encapsulated.
The extra `async_submitter` parameter on `_hookexec` / `traced_hookexec` is avoidable because `PluginManager` already owns `self._async_submitter`. You can keep the async feature but localize this concern inside `PluginManager` by:
* Removing `async_submitter` from the `_hookexec` / `traced_hookexec` signatures.
* Using `self._async_submitter` when delegating to `self._inner_hookexec`.
This makes hook callers and tracing oblivious to the submitter again, reducing API surface and coupling.
For example:
```python
class PluginManager:
...
def _hookexec(
self,
hook_name: str,
normal_impls: Sequence[NormalImpl],
wrapper_impls: Sequence[WrapperImpl],
caller_kwargs: Mapping[str, object],
firstresult: bool,
) -> object | list[object]:
# single source of truth
async_submitter = self._async_submitter
return self._inner_hookexec(
hook_name,
normal_impls,
wrapper_impls,
caller_kwargs,
firstresult,
async_submitter,
)
```
And in `enable_tracing`:
```python
def enable_tracing(
self,
before: _BeforeTrace | None = None,
after: _AfterTrace | None = None,
) -> Callable[[], None]:
...
oldcall = self._inner_hookexec
def traced_hookexec(
hook_name: str,
normal_impls: Sequence[NormalImpl],
wrapper_impls: Sequence[WrapperImpl],
caller_kwargs: Mapping[str, object],
firstresult: bool,
) -> object | list[object]:
hook_impls: list[HookImpl] = [*normal_impls, *wrapper_impls]
before(hook_name, hook_impls, caller_kwargs)
outcome = Result.from_call(
lambda: oldcall(
hook_name,
normal_impls,
wrapper_impls,
caller_kwargs,
firstresult,
self._async_submitter, # use manager-owned submitter
)
)
after(outcome, hook_name, hook_impls, caller_kwargs)
return outcome.get_result()
self._inner_hookexec = traced_hookexec
...
```
With this change:
* `_hookexec` and `traced_hookexec` keep their thinner, pre-async signatures.
* Only `PluginManager` knows about `async_submitter`; callers of `_hookexec` don’t need to pass it around.
* The async behavior is preserved because `_inner_hookexec` still receives the submitter, but the plumbing is hidden behind `PluginManager`.
</issue_to_address>
### Comment 2
<location path="src/pluggy/_caller.py" line_range="34" />
<code_context>
_HookExec: TypeAlias = Callable[
- [str, Sequence[NormalImpl], Sequence[WrapperImpl], Mapping[str, object], bool],
+ [
</code_context>
<issue_to_address>
**issue (complexity):** Consider keeping `_HookExec` independent of `Submitter` and binding the async submitter once in a central factory to avoid signature churn and per-caller async state.
The added `Submitter` parameter on `_HookExec` and the per-caller `_async_submitter` field do introduce avoidable coupling and signature churn. You can keep all async functionality while simplifying by binding the `Submitter` once (e.g. in `PluginManager`) and restoring the original `_HookExec` surface.
Concretely:
1. **Keep `_HookExec` unaware of `Submitter`**
Change `_HookExec` back to the original signature and wrap the underlying implementation with a closure/`partial` that captures the `Submitter`:
```python
# restore _HookExec type
_HookExec: TypeAlias = Callable[
[str, Sequence[NormalImpl], Sequence[WrapperImpl], Mapping[str, object], bool],
"object | list[object]",
]
# in PluginManager (or wherever _hookexec is created)
from functools import partial
from ._async import Submitter
def _create_hookexec(base_hookexec: Callable[..., object | list[object]],
submitter: Submitter) -> _HookExec:
def hookexec(
name: str,
normal_impls: Sequence[NormalImpl],
wrapper_impls: Sequence[WrapperImpl],
kwargs: Mapping[str, object],
firstresult: bool,
) -> object | list[object]:
return base_hookexec(
name,
normal_impls,
wrapper_impls,
kwargs,
firstresult,
submitter, # async plumbing stays here
)
return hookexec
# usage in PluginManager:
self._submitter = Submitter()
self._hookexec = _create_hookexec(base_hookexec, self._submitter)
```
2. **Drop per-caller `async_submitter` attributes and parameters**
Let `NormalHookCaller` and `HistoricHookCaller` just hold a bound `_hookexec` that already encapsulates the `Submitter`. Their call sites then remain simple:
```python
class NormalHookCaller:
__slots__ = ("name", "spec", "_hookexec", "_normal_hookimpls", "_wrapper_hookimpls")
def __init__(
self,
name: str,
hook_execute: _HookExec,
specmodule_or_class: _Namespace | None = None,
spec_config: HookspecConfiguration | None = None,
) -> None:
self.name = name
self._hookexec = hook_execute
# ...
def __call__(self, **kwargs: object) -> Any:
self._verify_all_args_are_provided(kwargs)
firstresult = self.spec.config.firstresult if self.spec else False
return self._hookexec(
self.name,
self._normal_hookimpls.copy(),
self._wrapper_hookimpls.copy(),
kwargs,
firstresult,
)
```
```python
class HistoricHookCaller:
__slots__ = ("name", "spec", "_hookexec", "_hookimpls", "_call_history")
def __init__(
self,
name: str,
hook_execute: _HookExec,
specmodule_or_class: _Namespace,
spec_config: HookspecConfiguration,
) -> None:
self.name = name
self._hookexec = hook_execute
# ...
def call_historic(
self,
result_callback: Callable[[Any], None] | None = None,
kwargs: Mapping[str, object] | None = None,
) -> None:
kwargs = kwargs or {}
self.spec.verify_all_args_are_provided(kwargs)
self._call_history.append((kwargs, result_callback))
res = self._hookexec(self.name, self._hookimpls.copy(), [], kwargs, False)
# ...
```
3. **Simplify `SubsetHookCaller` to just delegate**
Once `_hookexec` is bound with the `Submitter`, `SubsetHookCaller` doesn’t need to reach into `orig._async_submitter` or propagate it:
```python
class SubsetHookCaller:
def __call__(self, **kwargs: object) -> Any:
orig = self._orig
assert isinstance(orig, NormalHookCaller)
if orig.spec:
orig.spec.verify_all_args_are_provided(kwargs)
firstresult = orig.spec.config.firstresult if orig.spec else False
return orig._hookexec(
self.name,
self._get_filtered(orig._normal_hookimpls),
self._get_filtered(orig._wrapper_hookimpls),
kwargs,
firstresult,
)
def call_historic(
self,
result_callback: Callable[[Any], None] | None = None,
kwargs: Mapping[str, object] | None = None,
) -> None:
orig = self._orig
assert isinstance(orig, HistoricHookCaller)
kwargs = kwargs or {}
orig.spec.verify_all_args_are_provided(kwargs)
orig._call_history.append((kwargs, result_callback))
res = orig._hookexec(self.name, self._get_filtered(orig._hookimpls), [], kwargs, False)
# ...
```
This keeps the async plumbing localized to the `PluginManager` (or central factory) that owns a single `Submitter`, avoids multiple instances, and restores a cleaner, decoupled hook-caller surface without changing behaviour.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| wrapper_impls: Sequence[WrapperImpl], | ||
| caller_kwargs: Mapping[str, object], | ||
| firstresult: bool, | ||
| async_submitter: Submitter, |
There was a problem hiding this comment.
issue (complexity): Consider removing the async_submitter parameter from the public/internal hookexec functions and instead using the manager-owned submitter inside PluginManager to keep the async wiring encapsulated.
The extra async_submitter parameter on _hookexec / traced_hookexec is avoidable because PluginManager already owns self._async_submitter. You can keep the async feature but localize this concern inside PluginManager by:
- Removing
async_submitterfrom the_hookexec/traced_hookexecsignatures. - Using
self._async_submitterwhen delegating toself._inner_hookexec.
This makes hook callers and tracing oblivious to the submitter again, reducing API surface and coupling.
For example:
class PluginManager:
...
def _hookexec(
self,
hook_name: str,
normal_impls: Sequence[NormalImpl],
wrapper_impls: Sequence[WrapperImpl],
caller_kwargs: Mapping[str, object],
firstresult: bool,
) -> object | list[object]:
# single source of truth
async_submitter = self._async_submitter
return self._inner_hookexec(
hook_name,
normal_impls,
wrapper_impls,
caller_kwargs,
firstresult,
async_submitter,
)And in enable_tracing:
def enable_tracing(
self,
before: _BeforeTrace | None = None,
after: _AfterTrace | None = None,
) -> Callable[[], None]:
...
oldcall = self._inner_hookexec
def traced_hookexec(
hook_name: str,
normal_impls: Sequence[NormalImpl],
wrapper_impls: Sequence[WrapperImpl],
caller_kwargs: Mapping[str, object],
firstresult: bool,
) -> object | list[object]:
hook_impls: list[HookImpl] = [*normal_impls, *wrapper_impls]
before(hook_name, hook_impls, caller_kwargs)
outcome = Result.from_call(
lambda: oldcall(
hook_name,
normal_impls,
wrapper_impls,
caller_kwargs,
firstresult,
self._async_submitter, # use manager-owned submitter
)
)
after(outcome, hook_name, hook_impls, caller_kwargs)
return outcome.get_result()
self._inner_hookexec = traced_hookexec
...With this change:
_hookexecandtraced_hookexeckeep their thinner, pre-async signatures.- Only
PluginManagerknows aboutasync_submitter; callers of_hookexecdon’t need to pass it around. - The async behavior is preserved because
_inner_hookexecstill receives the submitter, but the plumbing is hidden behindPluginManager.
| @@ -31,7 +32,14 @@ | |||
|
|
|||
|
|
|||
| _HookExec: TypeAlias = Callable[ | |||
There was a problem hiding this comment.
issue (complexity): Consider keeping _HookExec independent of Submitter and binding the async submitter once in a central factory to avoid signature churn and per-caller async state.
The added Submitter parameter on _HookExec and the per-caller _async_submitter field do introduce avoidable coupling and signature churn. You can keep all async functionality while simplifying by binding the Submitter once (e.g. in PluginManager) and restoring the original _HookExec surface.
Concretely:
- Keep
_HookExecunaware ofSubmitter
Change_HookExecback to the original signature and wrap the underlying implementation with a closure/partialthat captures theSubmitter:
# restore _HookExec type
_HookExec: TypeAlias = Callable[
[str, Sequence[NormalImpl], Sequence[WrapperImpl], Mapping[str, object], bool],
"object | list[object]",
]
# in PluginManager (or wherever _hookexec is created)
from functools import partial
from ._async import Submitter
def _create_hookexec(base_hookexec: Callable[..., object | list[object]],
submitter: Submitter) -> _HookExec:
def hookexec(
name: str,
normal_impls: Sequence[NormalImpl],
wrapper_impls: Sequence[WrapperImpl],
kwargs: Mapping[str, object],
firstresult: bool,
) -> object | list[object]:
return base_hookexec(
name,
normal_impls,
wrapper_impls,
kwargs,
firstresult,
submitter, # async plumbing stays here
)
return hookexec
# usage in PluginManager:
self._submitter = Submitter()
self._hookexec = _create_hookexec(base_hookexec, self._submitter)- Drop per-caller
async_submitterattributes and parameters
LetNormalHookCallerandHistoricHookCallerjust hold a bound_hookexecthat already encapsulates theSubmitter. Their call sites then remain simple:
class NormalHookCaller:
__slots__ = ("name", "spec", "_hookexec", "_normal_hookimpls", "_wrapper_hookimpls")
def __init__(
self,
name: str,
hook_execute: _HookExec,
specmodule_or_class: _Namespace | None = None,
spec_config: HookspecConfiguration | None = None,
) -> None:
self.name = name
self._hookexec = hook_execute
# ...
def __call__(self, **kwargs: object) -> Any:
self._verify_all_args_are_provided(kwargs)
firstresult = self.spec.config.firstresult if self.spec else False
return self._hookexec(
self.name,
self._normal_hookimpls.copy(),
self._wrapper_hookimpls.copy(),
kwargs,
firstresult,
)class HistoricHookCaller:
__slots__ = ("name", "spec", "_hookexec", "_hookimpls", "_call_history")
def __init__(
self,
name: str,
hook_execute: _HookExec,
specmodule_or_class: _Namespace,
spec_config: HookspecConfiguration,
) -> None:
self.name = name
self._hookexec = hook_execute
# ...
def call_historic(
self,
result_callback: Callable[[Any], None] | None = None,
kwargs: Mapping[str, object] | None = None,
) -> None:
kwargs = kwargs or {}
self.spec.verify_all_args_are_provided(kwargs)
self._call_history.append((kwargs, result_callback))
res = self._hookexec(self.name, self._hookimpls.copy(), [], kwargs, False)
# ...- Simplify
SubsetHookCallerto just delegate
Once_hookexecis bound with theSubmitter,SubsetHookCallerdoesn’t need to reach intoorig._async_submitteror propagate it:
class SubsetHookCaller:
def __call__(self, **kwargs: object) -> Any:
orig = self._orig
assert isinstance(orig, NormalHookCaller)
if orig.spec:
orig.spec.verify_all_args_are_provided(kwargs)
firstresult = orig.spec.config.firstresult if orig.spec else False
return orig._hookexec(
self.name,
self._get_filtered(orig._normal_hookimpls),
self._get_filtered(orig._wrapper_hookimpls),
kwargs,
firstresult,
)
def call_historic(
self,
result_callback: Callable[[Any], None] | None = None,
kwargs: Mapping[str, object] | None = None,
) -> None:
orig = self._orig
assert isinstance(orig, HistoricHookCaller)
kwargs = kwargs or {}
orig.spec.verify_all_args_are_provided(kwargs)
orig._call_history.append((kwargs, result_callback))
res = orig._hookexec(self.name, self._get_filtered(orig._hookimpls), [], kwargs, False)
# ...This keeps the async plumbing localized to the PluginManager (or central factory) that owns a single Submitter, avoids multiple instances, and restores a cleaner, decoupled hook-caller surface without changing behaviour.
RonnyPfannschmidt
force-pushed
the
refactor/project-spec
branch
from
July 24, 2026 16:38
dec8c94 to
3ce85b3
Compare
RonnyPfannschmidt
force-pushed
the
refactor/async-submitter
branch
from
July 24, 2026 16:38
6394496 to
d23ca05
Compare
RonnyPfannschmidt
force-pushed
the
refactor/project-spec
branch
from
July 24, 2026 16:54
3ce85b3 to
d6217b7
Compare
RonnyPfannschmidt
force-pushed
the
refactor/async-submitter
branch
from
July 24, 2026 16:54
d23ca05 to
3910791
Compare
RonnyPfannschmidt
force-pushed
the
refactor/project-spec
branch
from
August 12, 2026 10:50
d6217b7 to
106949c
Compare
RonnyPfannschmidt
force-pushed
the
refactor/async-submitter
branch
from
August 12, 2026 10:50
3910791 to
a2e53c6
Compare
RonnyPfannschmidt
force-pushed
the
refactor/project-spec
branch
from
August 12, 2026 10:57
106949c to
b74f215
Compare
RonnyPfannschmidt
force-pushed
the
refactor/async-submitter
branch
from
August 12, 2026 10:57
a2e53c6 to
c437360
Compare
Complete design step 07: - New _async module with a persistent Submitter: maybe_submit awaits awaitable hook results while active and passes them through otherwise (await-me-maybe); require_await hard-fails outside async context; async_generator_to_sync helps wrappers consume async generators. - Submitter.run uses a sentinel so legitimate None returns work (fixes the try-claude footgun) and forwards await failures to the submission site inside the worker greenlet. - PluginManager owns the Submitter and threads it through the callers and _hookexec into _multicall - activation is purely Submitter.run, no _inner_hookexec monkeypatching. await pm.run_async(func) is the public entry point; nested runs raise. - Packaging: new pluggy[async] extra depending on greenlet; greenlet added to the testing group and types-greenlet to the mypy hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RonnyPfannschmidt
force-pushed
the
refactor/project-spec
branch
from
August 12, 2026 11:14
b74f215 to
95512c2
Compare
RonnyPfannschmidt
force-pushed
the
refactor/async-submitter
branch
from
August 12, 2026 11:14
c437360 to
76f5114
Compare
This was referenced Aug 12, 2026
require_await raised before awaiting, so the awaitable handed to it was never awaited and never closed. async_generator_to_sync calls it as require_await(async_gen.__anext__()), which meant the orphaned coroutine was created inside the library and no caller could reach it to clean up. CPython emitted "RuntimeWarning: coroutine method 'asend' ... was never awaited" while finalizing the async generator; the pytest config's filterwarnings = error turned that into an exception inside __del__, surfacing as an unraisable that failed the 3.13 and 3.14 jobs. require_await now takes ownership of the awaitable either way, closing it before raising when inactive. This matches maybe_submit, which hands the awaitable back to the caller when inactive rather than dropping it. Use getattr for close since Awaitable does not guarantee it. test_require_await_outside_context papered over this with a manual close(); it now asserts the coroutine was closed, which fails without the fix on every supported version rather than only on 3.13+. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
for more information, see https://pre-commit.ci
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
1 participant
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.
Review PR — step 7 of 7.
This PR targets the previous step's branch, so its diff is only this step's change. Review happens here. The corresponding upstream PR, which is the one that actually merges, is pytest-dev#710.
Merges happen upstream one step at a time, bottom-up. When step 7 lands upstream, this PR is closed and the rest of the stack is rebased onto the new
main.refactor/split-hook-modulesrefactor/configuration-objectsrefactor/markers-attach-configrefactor/hookimpl-wrapper-typesrefactor/hookcaller-and-executionrefactor/project-specrefactor/async-submitterChain step 07 of the internal-refactoring series (design/07-async-submitter.md).
Persistent greenlet Submitter threaded through callers into _multicall; await pm.run_async(...) awaits awaitable hook results (await-me-maybe outside it); new pluggy[async] extra.
Stacked on the step-06 chain PR.
🤖 Generated with Claude Code
Summary by Sourcery
Add greenlet-based async support to pluggy hooks and plugin manager, allowing awaitable hook implementations to be transparently awaited via PluginManager.run_async while preserving existing synchronous behavior.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: