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

Encapsulate spec definitions with a class #57

Merged
merged 2 commits into from
Aug 19, 2018

Conversation

goodboy
Copy link
Contributor

@goodboy goodboy commented Jul 7, 2017

Allows for easier introspection of spec definitions including function
signatures and hook options. Originally introduced to address #47 #15 and #43 which
requires keeping track of spec default arguments values.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, but the issue number you mention (#47) is about the README. 😉

@goodboy
Copy link
Contributor Author

goodboy commented Jul 8, 2017

@nicoddemus hah nice catch.
Yeah it's actually #15 and the accompanying PR #43.
I'll change the commit msg.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspekt if i had my computer on me I'd find anissie

Something about it irks me

@goodboy
Copy link
Contributor Author

goodboy commented Jul 10, 2017

@RonnyPfannschmidt yeah if you find anything let me know.

This isn't really all that important if you don't like it.
I just thought I'd grab the handy stuff from #43 which might make somone's life easier down the road.

@RonnyPfannschmidt
Copy link
Member

well, i fail to see it atm, lets go ahead with it

@goodboy
Copy link
Contributor Author

goodboy commented Aug 29, 2017

@RonnyPfannschmidt we can always leave this sitting until we need it.
It's really only useful if we plan to introspect hookspec func signatures in the future.

@goodboy
Copy link
Contributor Author

goodboy commented Aug 8, 2018

@fschulze take a look at this if you don't mind so we can figure out if it's going to meet your needs.
With this you can introspect spec args (and other things) using pm.hook.<hookname>.spec.

@fschulze
Copy link
Contributor

fschulze commented Aug 8, 2018

edit: sorry to have added this comment on this not entirely related PR, I thought I was replying on #170.

@tgoodlet

@fschulze take a look at this if you don't mind so we can figure out if it's going to meet your needs.
With this you can introspect spec args (and other things) using pm.hook..spec.

That again is from the host side, do you have anything in mind for the plugin side? Is that even possible?

I still think my proposal like this would be simpler:

Host side:

@hookspec(deprecated=True)
def myhost_oldhook(...):
    ...

@hookspec(replaces=myhost_oldhook)
def myhost_newhook(...):
    ...

Plugin side:

@hookimpl(optional=True)
def myhost_oldhook(...):
    ...

@hookimpl(optional=True)
def myhost_newhook(...):
    ...

Pluggy would then remove myhost_oldhook from the list. This way all the following work:

  • old host - old plugin
  • new host - old plugin (with deprecation warning)
  • old host - new plugin (because the hook impl is optional)
  • new host - new plugin (because pluggy removes the old hook impl from the list and new host only calls the new hook of the plugin)

@fschulze
Copy link
Contributor

fschulze commented Aug 8, 2018

This PR would make implementing my above proposal for #170 much simpler though.

@goodboy goodboy force-pushed the encapsulate_specs branch 3 times, most recently from 23dc109 to cf6b64a Compare August 10, 2018 16:31
pluggy/hooks.py Outdated
@@ -273,8 +267,10 @@ def __call__(self, *args, **kwargs):
if args:
raise TypeError("hook calling supports only keyword arguments")
assert not self.is_historic()
if self.argnames:
notincall = set(self.argnames) - set(["__multicall__"]) - set(kwargs.keys())
if self.spec:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for parity this should be self.spec.argnames

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call - this looks like a mistake/typo

pluggy/hooks.py Outdated
self.set_specification(specmodule_or_class, spec_opts)

def has_spec(self):
return self._specmodule_or_class is not None
return True if self.spec is not None else False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just return self.spec is not None? Much easier to read than the trinary operator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, indeed!

@goodboy
Copy link
Contributor Author

goodboy commented Aug 16, 2018

@RonnyPfannschmidt @fschulze changes addressed!

Btw @fschulze we've been having a discussion on gitter regarding the original problem.

Allows for easier introspection of spec definitions including function
signatures and hook options. Originally introduced to address pytest-dev#15 and
the accompanying PR (pytest-dev#43) which requires keeping track of spec default
arguments values.
@fschulze
Copy link
Contributor

@tgoodlet the conclusion of that discussion leads to my proposal again to allow deprecation of old hooks without double calls

@goodboy
Copy link
Contributor Author

goodboy commented Aug 16, 2018

@fschulze yup just keeping you in the loop :)

@nicoddemus
Copy link
Member

What else is needed to merge this?

@@ -215,10 +217,10 @@ def _verify_hook(self, hook, hookimpl):
"Plugin %r\nhook %r\nhistoric incompatible to hookwrapper"
% (hookimpl.plugin_name, hook.name),
)
if hook.warn_on_impl:
_warn_for_function(hook.warn_on_impl, hookimpl.function)
if hook.spec.warn_on_impl:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there'll be a bug here if there's no spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, nm. This is never called unless a spec exists.

@goodboy
Copy link
Contributor Author

goodboy commented Aug 19, 2018

@nicoddemus do you think we should document this as well or just leave as an internal until more people ask for it?

@goodboy goodboy merged commit 1da70c9 into pytest-dev:master Aug 19, 2018
@nicoddemus
Copy link
Member

@nicoddemus do you think we should document this as well or just leave as an internal until more people ask for it?

I think we can keep this an internal detail for now (sorry for the delay)

bors bot referenced this pull request in mozilla/normandy Oct 22, 2018
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 &lt;https://github.com/pytest-dev/pluggy/issues/177&gt;`_: Add ``get_hookimpls()`` method to hook callers.



Trivial/Internal Changes
------------------------

- `165 &lt;https://github.com/pytest-dev/pluggy/issues/165&gt;`_: Add changelog in long package description and documentation.


- `172 &lt;https://github.com/pytest-dev/pluggy/issues/172&gt;`_: Add a test exemplifying the opt-in nature of spec defined args.


- `57 &lt;https://github.com/pytest-dev/pluggy/issues/57&gt;`_: 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&#39;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 <[email protected]>
bors bot referenced this pull request in mozilla/normandy Oct 23, 2018
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 &lt;https://github.com/pytest-dev/pluggy/issues/177&gt;`_: Add ``get_hookimpls()`` method to hook callers.



Trivial/Internal Changes
------------------------

- `165 &lt;https://github.com/pytest-dev/pluggy/issues/165&gt;`_: Add changelog in long package description and documentation.


- `172 &lt;https://github.com/pytest-dev/pluggy/issues/172&gt;`_: Add a test exemplifying the opt-in nature of spec defined args.


- `57 &lt;https://github.com/pytest-dev/pluggy/issues/57&gt;`_: 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&#39;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 <[email protected]>
Co-authored-by: Mike Cooper <[email protected]>
takirala referenced this pull request in d2iq-archive/dcos-commons Dec 18, 2018
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

-   `#&#8203;177 <https://github.com/pytest-dev/pluggy/issues/177>`\_: Add `get_hookimpls()` method to hook callers.

## Trivial/Internal Changes

-   `#&#8203;165 <https://github.com/pytest-dev/pluggy/issues/165>`\_: Add changelog in long package description and documentation.

-   `#&#8203;172 <https://github.com/pytest-dev/pluggy/issues/172>`\_: Add a test exemplifying the opt-in nature of spec defined args.

-   `#&#8203;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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants