Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for custom hook callers? #151

Open
goodboy opened this issue May 22, 2018 · 9 comments
Open

Allow for custom hook callers? #151

goodboy opened this issue May 22, 2018 · 9 comments

Comments

@goodboy
Copy link
Contributor

goodboy commented May 22, 2018

In #133 there was a request to be able to provide a custom _HookCaller (in this case in order to be able to warn on deprecated hook calls - since addressed via #138).

This feature may be quite useful in the future not only in its ability to allow users to customize the pluggy runtime but also if we want to eventually support async / coroutine based plugins (this can be done loop agnostic these days with something like multio and/or trio-asyncio).

I'd like to get feedback on this.

To start it would mean that we would make a simple change where PluginManager can take a hook_caller_type as input during instantiation.

@RonnyPfannschmidt
Copy link
Member

i propose deferring this until we can experiment against trio and asyncio

based on initial example i'd absolutely avoid multio

@RonnyPfannschmidt
Copy link
Member

oh - we might want to investigate removing multi-call by moving it into a hook-caller subclass/mixin

@nicoddemus
Copy link
Member

nicoddemus commented Jun 5, 2018

How about multiple hook callers? At work we might need to use a pluggy hook call system identical to pluggy, but also capable of loading hook implementations from shared c++ libraries.

cc @williamjamir

@goodboy
Copy link
Contributor Author

goodboy commented Jun 5, 2018

Love both of those ideas!

@RonnyPfannschmidt
Copy link
Member

@nicoddemus sounds like overkill a hook parser or a wrapper sounds more viable there

@njsmith
Copy link

njsmith commented Jun 13, 2018

It's already possible to define a synchronous hook that internally does trio.run(something_async) or asyncio.run(something_async). To get extra value from async hooks, I think you'd need to somehow "expose" the async-ness to the caller, so that the caller can be async, and do things like:

async def run_test():
    await run_hooks("setup-fixtures")
    await run_hooks("run-test")
    await run_hooks("teardown-fixtures")

trio.run(run_test)

so that multiple hooks are running under the same event loop.

In principle I'm guessing this wouldn't be too complicated. But there's not much point unless one of pluggy's users (like pytest) is interested in using it in this mode.

@justanr
Copy link

justanr commented Jun 13, 2018

@njsmith I've toyed with this idea and what I ended coming up with is that hookimpls essentially become coroutine factories and the caller awaits their results, that way you can call from it inside another coroutine without pissing off the event loop (not sure how trio or curio handle the equivalent of loop.run_until_complete from inside a running coroutine, but I imagine they don't take kindly to it).

I never expanded it beyond toy examples so I'm not sure what shortcomings I missed beyond the caller needs to reimplement some of pluggy's filtering logic.

@ashwoods ashwoods mentioned this issue Sep 28, 2020
@RonnyPfannschmidt
Copy link
Member

i had a sync with @simonw on the weekend and i will try to incooperate the details into pluggy as i sprint on speedups this week

@lemon24
Copy link

lemon24 commented Oct 18, 2024

Hi, I maintain a feed reader library that supports plugins, and even if using pluggy would require some work-arounds for calling, I think the collection / discovery functionality is still more than worth it.

I want to describe the use cases I have in this comment, since it may help inform the design for custom hook callers.

  1. before feed update hook – first exception bubbles up; already supported

    for impl in impls:
        impl(reader, feed)
  2. after feed update hook – collect exceptions and raise as a group; Feature Suggestion: Collect multiple exceptions from hook implementations #419

    excs = []
    for impl in impls:
        try:
            impl(reader, feed)
        except Exception as e:
            excs.append(e)
    if excs:
        raise ExceptionGroup('...', excs)
  3. request hook – hook return value can replace one of the arguments

    request = ...
    for impl in impls:
        request = impl(request, **kwargs) or request
  4. response hook – maybe repeat action with hook return value

    request = ...
    response = send(request, **kwargs)
    
    for impl in impls:
        new_request = impl(response, request, **kwargs)
        if not new_request:
            continue
            
        request = new_request
        response = session.send(request, **kwargs)

In theory I think there are work-arounds for all cases, and with a bit of effort on the caller's part, without affecting plugin writers.

  • for (2), wrap HookImpl.function to append exceptions to a list
  • for (3), pass a reference to the request, and instead of returning a value, update the same reference; again, this can be done by a wrapper (decorator kind, not pluggy kind) over the original hook function (but accepting fewer arguments may need to be re-implemented in the wrapper)
  • (4) is a more complicated version of (3); presumably the "set reference" logic can also decide whether to re-run the action or not

AFAICT, supporting hook wrapper versions of the above may prove difficult, both in pluggy and in the work-around (lots of semantics to be decided).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants