Repository navigation
Deprecate __multicall__ support #23
Description
As per the comment in pluggy._MultiCall we should be able to drop the line here. Grepping the pytest sources reveals that only a few spots still reference it.
This change also allows us to remove the recursion induced by _wrapped_call() getting passed _MultiCall.execute. This means we can remove _wrapped_call and simply loop through hookwrapper pre and post yield calls at the beginning and end of execute(). I'd actually be interested to see if contextlib.contextmanager wrapping could be used directly for this.
Metadata
Metadata
Assignees
Labels
No labels
Activity
RonnyPfannschmidt commentedon Sep 13, 2016
it exists solely for backward compatibility
at the pytest sprint we discussed that instead of just removing it,
we should have a separate faster implementation without it and choose based on the hook
at the pytest sprint we discussed that instead of just removing it,
we should have a separate faster implementation without it and choose based on the hooks
goodboy commentedon Sep 13, 2016
@RonnyPfannschmidt ok so if I implement this where do you want the check for
__multicall__support to be?RonnyPfannschmidt commentedon Sep 14, 2016
i dont care where it is ^^ it has to work reliable :)
hpk42 commentedon Sep 14, 2016
I guess it makes sense to check if multicall is used during registering a plugin and mark the hook caller accordingly. On instantiation of PluginManager one should be able to optionally specify a callback which receives information about plugin and hook which uses multicall so a warning can be added.
Then at hook calling time we can switch between the two implementations. The current one would be used for when multicall is used.
hpk42 commentedon Nov 11, 2016
Also it makes sense to get rid of the last
pytestusages of__multicall__instances besides.RonnyPfannschmidt commentedon Nov 11, 2016
we should probably put the deprecation-warning into pluggy using python warnings themselfes
as for pytest itself, a quick grin showed that only the testsuite has a few useages leftover
nicoddemus commentedon Nov 11, 2016
Oh really? I swear I have removed those. 😬
goodboy commentedon Nov 11, 2016
Gonna try and hack on this over the weekend.
goodboy commentedon Feb 16, 2017
@hpk42 I actually started this but it turns out the way that tracing is accomplished kind of makes it a bit tricky with
_MultiCallbeing statically assigned in thePluginManager._inner_hookexeclambda.So I wanted to propose:
_HookCalleris instantiated with it's own_MultiCalland we revert to aLegacyMultiCallonly when'__multicall__'is detected in ahookimplsignature (i.e. when_HookCaller._add_hookimpl()is called)_HookCallerinstead of globallyPluginManager.enable_tracing()method (which could now also accept an optionalhooknamearg) that does it globally but internally it will enable tracing on each_HookCaller.PluginManager._hookexec()and_inner_hookexecbecause multicall objects will be instantiated once per_HookCallerinstead of on every hook call.Let me know what you think.
[-]Drop __multicall__ support[/-][+]Deprecate __multicall__ support[/+]Deprecate __multicall__ support