-
Notifications
You must be signed in to change notification settings - Fork 124
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
Feature/get plugins for hook #177
Feature/get plugins for hook #177
Conversation
pluggy/manager.py
Outdated
def get_hookimpl(self, name): | ||
""" Return a list of all hook implementations (HookImpl) for a given hook. """ | ||
hc = getattr(self.hook, name) | ||
return hc._wrappers + hc._nonwrappers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the part dealing with private attributes should be delegated to the hookcaller
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially did like this, but I saw other functions using also the _wrappers+_nonwrappers
pattern and decided to follow it.
Is it ok to refactor them as well to keep coherence?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like i missed the introduction of the pattern, lets do this as a follow-up issue so this one can be self-contained an in line with the current coding standard, we can resolve the pattern as a separate pr after its merit is taken into account
so lets keep yours as is 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok!
Another thing - if hookcaller can give you the hookimpl, then maybe it is more general if we can just get the caller from pluginmanager and then call get_hookimpl()
on the returned caller.
For getting the caller, we have two options:
- the plugin manager to have an additional method
hook_caller(name)
. Reads nice and simple. - or actually just use
subset_hook_caller
withremove_plugins
asNone
or[]
. It actually just returns the original caller. No need to introduce another function, but does not read as nice (running this subset method to actually get the whole thing, not the subset)
I think I prefer the first option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for now i wont explore the domain in detail as i lack personal time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized you can get the hookcaller just by pluginmanager.hook.<name>
. Even better, then I will just add the get_hookimpl
to the hook caller, it should be enough.
pluggy/hooks.py
Outdated
@@ -233,6 +233,9 @@ def remove(wrappers): | |||
if remove(self._nonwrappers) is None: | |||
raise ValueError("plugin %r not found" % (plugin,)) | |||
|
|||
def get_hookimpl(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semantics are bit off since there is of course a collection of callback functions.
I would go with get_hookimpls()
.
Also since in pluggy.manager
it seems every use of _wrappers
and _nonwrappers
is a concatenation,
>>> git grep _nonwrappers (git)-[master]
pluggy/hooks.py: self._nonwrappers = []
pluggy/hooks.py: if remove(self._nonwrappers) is None:
pluggy/hooks.py: methods = self._nonwrappers
pluggy/hooks.py: return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
pluggy/hooks.py: res = self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
pluggy/hooks.py: old = list(self._nonwrappers), list(self._wrappers)
pluggy/hooks.py: self._nonwrappers, self._wrappers = old
pluggy/manager.py: for hookfunction in hc._wrappers + hc._nonwrappers:
pluggy/manager.py: for hookimpl in hook._wrappers + hook._nonwrappers:
pluggy/manager.py: for hookimpl in orig._wrappers + orig._nonwrappers:
I'd just convert all lines containing hc._wrappers + hc._nonwrappers
to use this call now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with both points, seems harmless as the PR is quite small as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So currently the get_hookimpls()
is always returning a list with both wrappers and nonwrappers.
To replace, for instance, old = list(self._nonwrappers), list(self._wrappers)
, then we would have to possibly add an option to return them separately.
I cannot think of a very good way of doing it, so for now at least I won't touch this specific part. And it is not that bad as it is accessing its own private variable at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
turns out to be strictly dependent on this order "nonwrappers then wrappers". I changed get_hookimpls()
to be like this, hoping there will be no side-effects in the other ones that had the original order (tests are passing it at least).
@Sup3rGeo nice work 👍 @RonnyPfannschmidt @nicoddemus this is touching on some stuff I really don't like about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the small comments, we also need a CHANGELOG entry. 👍
pluggy/hooks.py
Outdated
@@ -233,6 +233,9 @@ def remove(wrappers): | |||
if remove(self._nonwrappers) is None: | |||
raise ValueError("plugin %r not found" % (plugin,)) | |||
|
|||
def get_hookimpl(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with both points, seems harmless as the PR is quite small as is.
For the changelog, this PR does not have a related issue. Should I use the PR id then? |
Yes, thanks! |
testing/test_pluginmanager.py
Outdated
hookimpls = pm.hook.he_method1.get_hookimpls() | ||
hook_plugins = [item.plugin for item in hookimpls] | ||
|
||
assert plugin1 in hook_plugins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took the liberty of improving this check a bit. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @Sup3rGeo, thanks!
Hi guys, is there anything left to be done in this PR? |
thanks for the reminder |
Late as always..but nice job @Sup3rGeo! |
1594: Scheduled weekly dependency update for week 42 r=mythmon a=pyup-bot ### Update [botocore](https://pypi.org/project/botocore) from **1.12.23** to **1.12.28**. <details> <summary>Changelog</summary> ### 1.12.28 ``` ======= * api-change:``workspaces``: Update workspaces client to latest version * api-change:``ssm``: Update ssm client to latest version ``` ### 1.12.27 ``` ======= * api-change:``medialive``: Update medialive client to latest version * api-change:``route53``: Update route53 client to latest version * api-change:``appstream``: Update appstream client to latest version ``` ### 1.12.26 ``` ======= * api-change:``events``: Update events client to latest version * api-change:``apigateway``: Update apigateway client to latest version ``` ### 1.12.25 ``` ======= * api-change:``glue``: Update glue client to latest version * api-change:``lightsail``: Update lightsail client to latest version * api-change:``resource-groups``: Update resource-groups client to latest version ``` ### 1.12.24 ``` ======= * api-change:``rds``: Update rds client to latest version * api-change:``lambda``: Update lambda client to latest version * api-change:``servicecatalog``: Update servicecatalog client to latest version ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/botocore - Changelog: https://pyup.io/changelogs/botocore/ - Repo: https://github.com/boto/botocore </details> ### Update [pluggy](https://pypi.org/project/pluggy) from **0.7.1** to **0.8.0**. <details> <summary>Changelog</summary> ### 0.8.0 ``` ========================= Features -------- - `177 <https://github.com/pytest-dev/pluggy/issues/177>`_: Add ``get_hookimpls()`` method to hook callers. Trivial/Internal Changes ------------------------ - `165 <https://github.com/pytest-dev/pluggy/issues/165>`_: Add changelog in long package description and documentation. - `172 <https://github.com/pytest-dev/pluggy/issues/172>`_: Add a test exemplifying the opt-in nature of spec defined args. - `57 <https://github.com/pytest-dev/pluggy/issues/57>`_: Encapsulate hook specifications in a type for easier introspection. ========= Changelog ========= .. towncrier release notes start ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/pluggy - Changelog: https://pyup.io/changelogs/pluggy/ - Repo: https://github.com/pytest-dev/pluggy </details> ### Update [urllib3](https://pypi.org/project/urllib3) from **1.23** to **1.24**. <details> <summary>Changelog</summary> ### 1.24 ``` ----------------- * Allow key_server_hostname to be specified when initializing a PoolManager to allow custom SNI to be overridden. (Pull 1449) * Test against Python 3.7 on AppVeyor. (Pull 1453) * Early-out ipv6 checks when running on App Engine. (Pull 1450) * Change ambiguous description of backoff_factor (Pull 1436) * Add ability to handle multiple Content-Encodings (Issue 1441 and Pull 1442) * Skip DNS names that can't be idna-decoded when using pyOpenSSL (Issue 1405). * Add a server_hostname parameter to HTTPSConnection which allows for overriding the SNI hostname sent in the handshake. (Pull 1397) * Drop support for EOL Python 2.6 (Pull 1429 and Pull 1430) * Fixed bug where responses with header Content-Type: message/* erroneously raised HeaderParsingError, resulting in a warning being logged. (Pull 1439) * Move urllib3 to src/urllib3 (Pull 1409) ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/urllib3 - Changelog: https://pyup.io/changelogs/urllib3/ - Docs: https://urllib3.readthedocs.io/ </details> ### Update [google-api-core](https://pypi.org/project/google-api-core) from **1.4.1** to **1.5.0**. *The bot wasn't able to find a changelog for this release. [Got an idea?](https://github.com/pyupio/changelogs/issues/new)* <details> <summary>Links</summary> - PyPI: https://pypi.org/project/google-api-core - Repo: https://github.com/GoogleCloudPlatform/google-cloud-python </details> ### Update [boto3](https://pypi.org/project/boto3) from **1.9.23** to **1.9.28**. <details> <summary>Changelog</summary> ### 1.9.28 ``` ====== * api-change:``workspaces``: [``botocore``] Update workspaces client to latest version * api-change:``ssm``: [``botocore``] Update ssm client to latest version ``` ### 1.9.27 ``` ====== * api-change:``medialive``: [``botocore``] Update medialive client to latest version * api-change:``route53``: [``botocore``] Update route53 client to latest version * api-change:``appstream``: [``botocore``] Update appstream client to latest version ``` ### 1.9.26 ``` ====== * api-change:``events``: [``botocore``] Update events client to latest version * api-change:``apigateway``: [``botocore``] Update apigateway client to latest version ``` ### 1.9.25 ``` ====== * api-change:``glue``: [``botocore``] Update glue client to latest version * api-change:``lightsail``: [``botocore``] Update lightsail client to latest version * api-change:``resource-groups``: [``botocore``] Update resource-groups client to latest version ``` ### 1.9.24 ``` ====== * api-change:``rds``: [``botocore``] Update rds client to latest version * api-change:``lambda``: [``botocore``] Update lambda client to latest version * api-change:``servicecatalog``: [``botocore``] Update servicecatalog client to latest version ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/boto3 - Changelog: https://pyup.io/changelogs/boto3/ - Repo: https://github.com/boto/boto3 </details> ### Update [djangorestframework](https://pypi.org/project/djangorestframework) from **3.8.2** to **3.9.0**. *The bot wasn't able to find a changelog for this release. [Got an idea?](https://github.com/pyupio/changelogs/issues/new)* <details> <summary>Links</summary> - PyPI: https://pypi.org/project/djangorestframework - Changelog: https://pyup.io/changelogs/djangorestframework/ - Homepage: https://www.django-rest-framework.org/ </details> ### Update [pytest](https://pypi.org/project/pytest) from **3.8.2** to **3.9.1**. *The bot wasn't able to find a changelog for this release. [Got an idea?](https://github.com/pyupio/changelogs/issues/new)* <details> <summary>Links</summary> - PyPI: https://pypi.org/project/pytest - Changelog: https://pyup.io/changelogs/pytest/ - Homepage: https://docs.pytest.org/en/latest/ </details> ### Update [requests](https://pypi.org/project/requests) from **2.19.1** to **2.20.0**. <details> <summary>Changelog</summary> ### 2.20.0 ``` ------------------- **Bugfixes** - Content-Type header parsing is now case-insensitive (e.g. charset=utf8 v Charset=utf8). - Fixed exception leak where certain redirect urls would raise uncaught urllib3 exceptions. - Requests removes Authorization header from requests redirected from https to http on the same hostname. (CVE-2018-18074) - `should_bypass_proxies` now handles URIs without hostnames (e.g. files). **Dependencies** - Requests now supports urllib3 v1.24. **Deprecations** - Requests has officially stopped support for Python 2.6. ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/requests - Changelog: https://pyup.io/changelogs/requests/ - Homepage: http://python-requests.org </details> Co-authored-by: pyup-bot <github-bot@pyup.io>
1594: Scheduled weekly dependency update for week 42 r=peterbe a=pyup-bot ### Update [botocore](https://pypi.org/project/botocore) from **1.12.23** to **1.12.28**. <details> <summary>Changelog</summary> ### 1.12.28 ``` ======= * api-change:``workspaces``: Update workspaces client to latest version * api-change:``ssm``: Update ssm client to latest version ``` ### 1.12.27 ``` ======= * api-change:``medialive``: Update medialive client to latest version * api-change:``route53``: Update route53 client to latest version * api-change:``appstream``: Update appstream client to latest version ``` ### 1.12.26 ``` ======= * api-change:``events``: Update events client to latest version * api-change:``apigateway``: Update apigateway client to latest version ``` ### 1.12.25 ``` ======= * api-change:``glue``: Update glue client to latest version * api-change:``lightsail``: Update lightsail client to latest version * api-change:``resource-groups``: Update resource-groups client to latest version ``` ### 1.12.24 ``` ======= * api-change:``rds``: Update rds client to latest version * api-change:``lambda``: Update lambda client to latest version * api-change:``servicecatalog``: Update servicecatalog client to latest version ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/botocore - Changelog: https://pyup.io/changelogs/botocore/ - Repo: https://github.com/boto/botocore </details> ### Update [pluggy](https://pypi.org/project/pluggy) from **0.7.1** to **0.8.0**. <details> <summary>Changelog</summary> ### 0.8.0 ``` ========================= Features -------- - `177 <https://github.com/pytest-dev/pluggy/issues/177>`_: Add ``get_hookimpls()`` method to hook callers. Trivial/Internal Changes ------------------------ - `165 <https://github.com/pytest-dev/pluggy/issues/165>`_: Add changelog in long package description and documentation. - `172 <https://github.com/pytest-dev/pluggy/issues/172>`_: Add a test exemplifying the opt-in nature of spec defined args. - `57 <https://github.com/pytest-dev/pluggy/issues/57>`_: Encapsulate hook specifications in a type for easier introspection. ========= Changelog ========= .. towncrier release notes start ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/pluggy - Changelog: https://pyup.io/changelogs/pluggy/ - Repo: https://github.com/pytest-dev/pluggy </details> ### Update [urllib3](https://pypi.org/project/urllib3) from **1.23** to **1.24**. <details> <summary>Changelog</summary> ### 1.24 ``` ----------------- * Allow key_server_hostname to be specified when initializing a PoolManager to allow custom SNI to be overridden. (Pull 1449) * Test against Python 3.7 on AppVeyor. (Pull 1453) * Early-out ipv6 checks when running on App Engine. (Pull 1450) * Change ambiguous description of backoff_factor (Pull 1436) * Add ability to handle multiple Content-Encodings (Issue 1441 and Pull 1442) * Skip DNS names that can't be idna-decoded when using pyOpenSSL (Issue 1405). * Add a server_hostname parameter to HTTPSConnection which allows for overriding the SNI hostname sent in the handshake. (Pull 1397) * Drop support for EOL Python 2.6 (Pull 1429 and Pull 1430) * Fixed bug where responses with header Content-Type: message/* erroneously raised HeaderParsingError, resulting in a warning being logged. (Pull 1439) * Move urllib3 to src/urllib3 (Pull 1409) ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/urllib3 - Changelog: https://pyup.io/changelogs/urllib3/ - Docs: https://urllib3.readthedocs.io/ </details> ### Update [google-api-core](https://pypi.org/project/google-api-core) from **1.4.1** to **1.5.0**. *The bot wasn't able to find a changelog for this release. [Got an idea?](https://github.com/pyupio/changelogs/issues/new)* <details> <summary>Links</summary> - PyPI: https://pypi.org/project/google-api-core - Repo: https://github.com/GoogleCloudPlatform/google-cloud-python </details> ### Update [boto3](https://pypi.org/project/boto3) from **1.9.23** to **1.9.28**. <details> <summary>Changelog</summary> ### 1.9.28 ``` ====== * api-change:``workspaces``: [``botocore``] Update workspaces client to latest version * api-change:``ssm``: [``botocore``] Update ssm client to latest version ``` ### 1.9.27 ``` ====== * api-change:``medialive``: [``botocore``] Update medialive client to latest version * api-change:``route53``: [``botocore``] Update route53 client to latest version * api-change:``appstream``: [``botocore``] Update appstream client to latest version ``` ### 1.9.26 ``` ====== * api-change:``events``: [``botocore``] Update events client to latest version * api-change:``apigateway``: [``botocore``] Update apigateway client to latest version ``` ### 1.9.25 ``` ====== * api-change:``glue``: [``botocore``] Update glue client to latest version * api-change:``lightsail``: [``botocore``] Update lightsail client to latest version * api-change:``resource-groups``: [``botocore``] Update resource-groups client to latest version ``` ### 1.9.24 ``` ====== * api-change:``rds``: [``botocore``] Update rds client to latest version * api-change:``lambda``: [``botocore``] Update lambda client to latest version * api-change:``servicecatalog``: [``botocore``] Update servicecatalog client to latest version ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/boto3 - Changelog: https://pyup.io/changelogs/boto3/ - Repo: https://github.com/boto/boto3 </details> ### Update [djangorestframework](https://pypi.org/project/djangorestframework) from **3.8.2** to **3.9.0**. *The bot wasn't able to find a changelog for this release. [Got an idea?](https://github.com/pyupio/changelogs/issues/new)* <details> <summary>Links</summary> - PyPI: https://pypi.org/project/djangorestframework - Changelog: https://pyup.io/changelogs/djangorestframework/ - Homepage: https://www.django-rest-framework.org/ </details> ### Update [pytest](https://pypi.org/project/pytest) from **3.8.2** to **3.9.1**. *The bot wasn't able to find a changelog for this release. [Got an idea?](https://github.com/pyupio/changelogs/issues/new)* <details> <summary>Links</summary> - PyPI: https://pypi.org/project/pytest - Changelog: https://pyup.io/changelogs/pytest/ - Homepage: https://docs.pytest.org/en/latest/ </details> ### Update [requests](https://pypi.org/project/requests) from **2.19.1** to **2.20.0**. <details> <summary>Changelog</summary> ### 2.20.0 ``` ------------------- **Bugfixes** - Content-Type header parsing is now case-insensitive (e.g. charset=utf8 v Charset=utf8). - Fixed exception leak where certain redirect urls would raise uncaught urllib3 exceptions. - Requests removes Authorization header from requests redirected from https to http on the same hostname. (CVE-2018-18074) - `should_bypass_proxies` now handles URIs without hostnames (e.g. files). **Dependencies** - Requests now supports urllib3 v1.24. **Deprecations** - Requests has officially stopped support for Python 2.6. ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/requests - Changelog: https://pyup.io/changelogs/requests/ - Homepage: http://python-requests.org </details> Co-authored-by: pyup-bot <github-bot@pyup.io> Co-authored-by: Mike Cooper <mythmon@gmail.com>
This PR contains the following updates: | Package | Update | Change | References | |---|---|---|---| | pluggy | minor | `==0.7.1` -> `==0.8.0` | [source](https://renovatebot.com/gh/pytest-dev/pluggy) | --- ### Release Notes <details> <summary>pytest-dev/pluggy</summary> ### [`v0.8.0`](https://renovatebot.com/gh/pytest-dev/pluggy/blob/master/CHANGELOG.rst#pluggy-080-2018-10-15) [Compare Source](https://renovatebot.com/gh/pytest-dev/pluggy/compare/0.7.1...0.8.0) ========================= ## Features - `#​177 <https://github.com/pytest-dev/pluggy/issues/177>`\_: Add `get_hookimpls()` method to hook callers. ## Trivial/Internal Changes - `#​165 <https://github.com/pytest-dev/pluggy/issues/165>`\_: Add changelog in long package description and documentation. - `#​172 <https://github.com/pytest-dev/pluggy/issues/172>`\_: Add a test exemplifying the opt-in nature of spec defined args. - `#​57 <https://github.com/pytest-dev/pluggy/issues/57>`\_: Encapsulate hook specifications in a type for easier introspection. </details> --- ### Renovate configuration :date: **Schedule**: At any time (no schedule defined). :vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied. :recycle: **Rebasing**: Whenever PR becomes conflicted, or if you modify the PR title to begin with "`rebase!`". :no_bell: **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- renovate-rebase -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://renovatebot.com/gh/marketplace/renovate). View repository job log [here](https://renovatebot.com/dashboard#mesosphere/dcos-commons).
Adds API function to retrieve HookImpl for a given hook name.
Created to support pytest-dev/pytest#3479