Repository navigation
How to best add and act on custom metadata for hookspec_opts and hookimpl_opts #133
Description
I'm looking to figure out the best way to deprecate a hook we have provided to end users and might be implemented in some plugins already. In a more general sense, I want to be able to add my own metadata to spec and impl opts and have ways of acting on that inside pluggy itself.
For the record, I have considered registering a try-first implementation or hook wrapper that raises the deprecation warning. And that's the easy answer, but I'd like to be able to include this information in documentation of the spec itself (potentially through a sphinx plugin or something similar) so inserting it into the spec_opts seemed like the wiser thing to do. From there, I'd also like to be able to act on it in the hook caller.
However, I found this difficult mostly because of that static calls to _HookCaller that required copying large amounts of code into a subclass. Some of it could be cut down -- for example _HookCaller.__call__ doesn't need to be overridden only _HookCaller._hookexec needs to be intercepted and wrapped.
I understand that I'm poking at things that are probably not intended to be poked at, or at least weren't considered open to being poked at. But is there potential for opening them up to poking?
Activity
goodboy commentedon Apr 16, 2018
@jaraco I'm a massive fan of some good poking 😄; as long as we've got tests most good ideas are greatly welcome!
A built-in hook deprecation system is something I think we can definitely consider though I'd like to get feedback from @nicoddemus and @RonnyPfannschmidt on your suggestions.
My thoughts:
MetadataHookspecMarker; I think it's totally fine to pass through extra opts via**kwargs(I'd say PR this in asap as it's non-invasive)DeprecatedHookCalleris very interesting, particularly subclassing just to addprecall()- this might be one of those motivating example to makepluggyself-hosting where we addpluggyhooks topluggyitself?InstrumentedManagerI totally think we should allow overloading the_HookCallertype and I think the addition ofhook_caller_class(I would go withhook_caller_type) is completely warrantedThanks for this feedback it's always nice to see some interest in extending the feature set 👍
justanr commentedon Apr 16, 2018
I'm curious what a self hosting pluggy looks like. I was thinking more along the lines of potential cooperative multiple inheritance than that route though.
Depending on how others feel, the earliest I could put together good PRs with tests would be this coming weekend.
goodboy commentedon May 17, 2018
@justanr given that @RonnyPfannschmidt added the deprecation support you desired directly, do we need to look at this any further?
The remaining features I can see being proposed here are:
HookspecMarkervia**kwargsPluginManagerto take in ahook_caller_typearg such that a user can implement a custom hook callerGiven that both of these requirements were a result of your need for the deprecation mark (which is now supported) would you still have reasons to move forward with these features?
@RonnyPfannschmidt @nicoddemus I personally think the latter is an interesting addition particularly if we ever do want to support async / coroutine hook implementations. For example, implementing an
AsyncHookCallerwithtriowould be trivial.justanr commentedon May 17, 2018
I don't have a direct need for after the depreciation PR went in. There's still potential use here, noted by your async suggestion and I'm willing to put the work in for the PRs if others think this is a good idea.
goodboy commentedon May 22, 2018
@justanr I've made #151 and #152 to follow up on this discussion.
Since the underlying need has been addressed I'm closing this.
Thanks again for the input!