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.

Sign up for GitHub

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

Jump to bottom

Allow cache_hash=True to be used when a custom __setstate__ is present #494

Closed
gabbard opened this issue Jan 28, 2019 · 2 comments · Fixed by #620
Closed

Allow cache_hash=True to be used when a custom __setstate__ is present #494

gabbard opened this issue Jan 28, 2019 · 2 comments · Fixed by #620
Labels
Bug

Comments

@gabbard
Copy link
Member

gabbard commented Jan 28, 2019

This is not currently possible (as of the merge of #489 ) because when cache_hash=True, we solve #482 by adding a __setstate__ to all classes which clears the cached hash code.

This is a somewhat unusual combination of needs, so I imagine this is very low priority. This issue is just here as a marker for anyone who runs into this problem in the future.
@gabbard gabbard mentioned this issue Jan 28, 2019
Merged
10 tasks
@wsanchez wsanchez added the Bug label Jan 30, 2019
bors bot referenced this issue in mozilla/normandy Mar 4, 2019
@bors @pyup-bot
Merge #1784
2f0a8c6 
1784: Scheduled weekly dependency update for week 09 r=peterbe a=pyup-bot






### Update [attrs](https://pypi.org/project/attrs) from **18.2.0** to **19.1.0**.


<details>
  <summary>Changelog</summary>
  
  
   ### 19.1.0
   ```
   -------------------

Backward-incompatible Changes
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

- Fixed a bug where deserialized objects with ``cache_hash=True`` could have incorrect hash code values.
  This change breaks classes with ``cache_hash=True`` when a custom ``__setstate__`` is present.
  An exception will be thrown when applying the ``attrs`` annotation to such a class.
  This limitation is tracked in issue `494 &lt;https://github.com/python-attrs/attrs/issues/494&gt;`_.
  `482 &lt;https://github.com/python-attrs/attrs/issues/482&gt;`_


Changes
^^^^^^^

- Add ``is_callable``, ``deep_iterable``, and ``deep_mapping`` validators.

  * ``is_callable``: validates that a value is callable
  * ``deep_iterable``: Allows recursion down into an iterable,
    applying another validator to every member in the iterable
    as well as applying an optional validator to the iterable itself.
  * ``deep_mapping``: Allows recursion down into the items in a mapping object,
    applying a key validator and a value validator to the key and value in every item.
    Also applies an optional validator to the mapping object itself.

  You can find them in the ``attr.validators`` package.
  `425 &lt;https://github.com/python-attrs/attrs/issues/425&gt;`_
- Fixed stub files to prevent errors raised by mypy&#39;s ``disallow_any_generics = True`` option.
  `443 &lt;https://github.com/python-attrs/attrs/issues/443&gt;`_
- Attributes with ``init=False`` now can follow after ``kw_only=True`` attributes.
  `450 &lt;https://github.com/python-attrs/attrs/issues/450&gt;`_
- ``attrs`` now has first class support for defining exception classes.

  If you define a class using ``attr.s(auto_exc=True)`` and subclass an exception, the class will behave like a well-behaved exception class including an appropriate ``__str__`` method, and all attributes additionally available in an ``args`` attribute.
  `500 &lt;https://github.com/python-attrs/attrs/issues/500&gt;`_
- Clarified documentation for hashing to warn that hashable objects should be deeply immutable (in their usage, even if this is not enforced).
  `503 &lt;https://github.com/python-attrs/attrs/issues/503&gt;`_


----
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/attrs
  - Changelog: https://pyup.io/changelogs/attrs/
  - Homepage: https://www.attrs.org/
</details>





### Update [botocore](https://pypi.org/project/botocore) from **1.12.101** to **1.12.106**.


<details>
  <summary>Changelog</summary>
  
  
   ### 1.12.106
   ```
   ========

* api-change:``ec2``: Update ec2 client to latest version
* api-change:``autoscaling-plans``: Update autoscaling-plans client to latest version
   ```
   
  
  
   ### 1.12.105
   ```
   ========

* api-change:``ssm``: Update ssm client to latest version
* api-change:``apigatewayv2``: Update apigatewayv2 client to latest version
* api-change:``alexaforbusiness``: Update alexaforbusiness client to latest version
* api-change:``application-autoscaling``: Update application-autoscaling client to latest version
   ```
   
  
  
   ### 1.12.104
   ```
   ========

* api-change:``waf-regional``: Update waf-regional client to latest version
* api-change:``waf``: Update waf client to latest version
   ```
   
  
  
   ### 1.12.103
   ```
   ========

* api-change:``discovery``: Update discovery client to latest version
* api-change:``organizations``: Update organizations client to latest version
* api-change:``resource-groups``: Update resource-groups client to latest version
* api-change:``opsworkscm``: Update opsworkscm client to latest version
* api-change:``pinpoint``: Update pinpoint client to latest version
* api-change:``mediaconvert``: Update mediaconvert client to latest version
* api-change:``cur``: Update cur client to latest version
   ```
   
  
  
   ### 1.12.102
   ```
   ========

* api-change:``elbv2``: Update elbv2 client to latest version
* api-change:``mediastore``: Update mediastore client to latest version
* api-change:``ce``: Update ce client to latest version
* api-change:``autoscaling``: Update autoscaling 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 [cffi](https://pypi.org/project/cffi) from **1.12.1** to **1.12.2**.


*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/cffi
  - Changelog: https://pyup.io/changelogs/cffi/
  - Docs: http://cffi.readthedocs.org
</details>





### Update [idna](https://pypi.org/project/idna) from **2.6** to **2.8**.


<details>
  <summary>Changelog</summary>
  
  
   ### 2.8
   ```
   ++++++++++++++++

- Update to Unicode 11.0.0.
- Provide more specific exceptions for some malformed labels.
   ```
   
  
  
   ### 2.7
   ```
   ++++++++++++++++

- Update to Unicode 10.0.0.
- No longer accepts dot-prefixed domains (e.g. &quot;.example&quot;) as valid.
  This is to be more conformant with the UTS 46 spec. Users should
  strip dot prefixes from domains before processing.
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/idna
  - Changelog: https://pyup.io/changelogs/idna/
  - Repo: https://github.com/kjd/idna
</details>





### Update [pyflakes](https://pypi.org/project/pyflakes) from **2.1.0** to **2.1.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/pyflakes
  - Changelog: https://pyup.io/changelogs/pyflakes/
  - Repo: https://github.com/PyCQA/pyflakes
</details>





### Update [google-api-core](https://pypi.org/project/google-api-core) from **1.7.0** to **1.8.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 [google-cloud-core](https://pypi.org/project/google-cloud-core) from **0.28.1** to **0.29.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/google-cloud-core
  - Repo: https://github.com/GoogleCloudPlatform/google-cloud-python
</details>





### Update [protobuf](https://pypi.org/project/protobuf) from **3.6.1** to **3.7.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/protobuf
  - Changelog: https://pyup.io/changelogs/protobuf/
  - Repo: https://github.com/protocolbuffers/protobuf/releases
  - Homepage: https://developers.google.com/protocol-buffers/
</details>





### Update [boto3](https://pypi.org/project/boto3) from **1.9.101** to **1.9.106**.


<details>
  <summary>Changelog</summary>
  
  
   ### 1.9.106
   ```
   =======

* api-change:``ec2``: [``botocore``] Update ec2 client to latest version
* api-change:``autoscaling-plans``: [``botocore``] Update autoscaling-plans client to latest version
   ```
   
  
  
   ### 1.9.105
   ```
   =======

* api-change:``ssm``: [``botocore``] Update ssm client to latest version
* api-change:``apigatewayv2``: [``botocore``] Update apigatewayv2 client to latest version
* api-change:``alexaforbusiness``: [``botocore``] Update alexaforbusiness client to latest version
* api-change:``application-autoscaling``: [``botocore``] Update application-autoscaling client to latest version
   ```
   
  
  
   ### 1.9.104
   ```
   =======

* api-change:``waf-regional``: [``botocore``] Update waf-regional client to latest version
* api-change:``waf``: [``botocore``] Update waf client to latest version
   ```
   
  
  
   ### 1.9.103
   ```
   =======

* api-change:``discovery``: [``botocore``] Update discovery client to latest version
* api-change:``organizations``: [``botocore``] Update organizations client to latest version
* api-change:``resource-groups``: [``botocore``] Update resource-groups client to latest version
* api-change:``opsworkscm``: [``botocore``] Update opsworkscm client to latest version
* api-change:``pinpoint``: [``botocore``] Update pinpoint client to latest version
* api-change:``mediaconvert``: [``botocore``] Update mediaconvert client to latest version
* api-change:``cur``: [``botocore``] Update cur client to latest version
   ```
   
  
  
   ### 1.9.102
   ```
   =======

* api-change:``elbv2``: [``botocore``] Update elbv2 client to latest version
* api-change:``mediastore``: [``botocore``] Update mediastore client to latest version
* api-change:``ce``: [``botocore``] Update ce client to latest version
* api-change:``autoscaling``: [``botocore``] Update autoscaling 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 [django-cors-headers](https://pypi.org/project/django-cors-headers) from **2.4.0** to **2.4.1**.


<details>
  <summary>Changelog</summary>
  
  
   ### 2.4.1
   ```
   ------------------

* Fix ``DeprecationWarning`` from importing ``collections.abc.Sequence`` on
  Python 3.7.
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/django-cors-headers
  - Changelog: https://pyup.io/changelogs/django-cors-headers/
  - Repo: https://github.com/ottoyiu/django-cors-headers
</details>





### Update [django-mozilla-product-details](https://pypi.org/project/django-mozilla-product-details) from **0.13** to **0.13.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/django-mozilla-product-details
  - Repo: https://github.com/mozilla/django-product-details/
</details>





### Update [djangorestframework](https://pypi.org/project/djangorestframework) from **3.9.1** to **3.9.2**.


*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 [jsonschema](https://pypi.org/project/jsonschema) from **3.0.0** to **3.0.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/jsonschema
  - Changelog: https://pyup.io/changelogs/jsonschema/
  - Repo: https://github.com/Julian/jsonschema
</details>





### Update [pytest-django](https://pypi.org/project/pytest-django) from **3.4.7** to **3.4.8**.


<details>
  <summary>Changelog</summary>
  
  
   ### 3.4.8
   ```
   ------------------

Bugfixes
^^^^^^^^

* Fix DB renaming fixture for Multi-DB environment with SQLite (679)
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pytest-django
  - Changelog: https://pyup.io/changelogs/pytest-django/
  - Docs: https://pytest-django.readthedocs.io/
</details>





### Update [google-cloud-storage](https://pypi.org/project/google-cloud-storage) from **1.13.0** to **1.14.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-cloud-storage
  - Repo: https://github.com/GoogleCloudPlatform/google-cloud-python
</details>







Co-authored-by: pyup-bot <[email protected]>
@pganssle
Copy link
Member

pganssle commented Jan 8, 2020

How about solving this (albeit mostly theoretical) issue by clearing the hash cache after invoking the custom __setstate__, like so (not including changes from #612 for simplicity, and there are lots of variations on how to implement this):

original_setstate = getattr(cls, __setstate__, lambda s, _: None)
def cache_hash_set_state(chss_self, _):
    original_setstate(chss_self, _)
    setattr(chss_self, _hash_cache_field, None)

If you consider the hash cache field to be an implementation detail owned by attrs, I think that likely the custom __setstate__ shouldn't be touching it anyway, so this seems pretty safe. If you want a custom __setstate__ that also has custom behavior involving the hash caching, you can implement a custom __hash__ instead of using cache_hash=True.

@gabbard
Copy link
Member Author

gabbard commented Jan 8, 2020

@pganssle : It has been a long time since I've touched this code, so I could be mistaken, but I believe your solution should work fine.

pganssle added a commit to pganssle/attrs that referenced this issue Jan 13, 2020
@pganssle
Implement __reduce__ with cache_hash
0285d3d 
This fixes GH issue python-attrs#613 and python-attrs#494. It turns out that the hash
cache-clearing implementation for non-slots classes was flawed and never
quite worked properly. This switches away from using __setstate__ and
instead adds a custom __reduce__ that removes the cached hash value from
the default serialized output.

This commit also refactors some of the tests a bit, to try and more
cleanly organize the tests related to this issue.
@pganssle pganssle mentioned this issue Jan 13, 2020
Closed
4 tasks
pganssle added a commit to pganssle/attrs that referenced this issue Jan 13, 2020
@pganssle
Implement __reduce__ with cache_hash
c5b1dfd 
This fixes GH issue python-attrs#613 and python-attrs#494. It turns out that the hash
cache-clearing implementation for non-slots classes was flawed and never
quite worked properly. This switches away from using __setstate__ and
instead adds a custom __reduce__ that removes the cached hash value from
the default serialized output.

This commit also refactors some of the tests a bit, to try and more
cleanly organize the tests related to this issue.
@pganssle pganssle mentioned this issue Jan 14, 2020
Closed
4 tasks
pganssle added a commit to pganssle/attrs that referenced this issue Jan 14, 2020
@pganssle
Implement __reduce__ with cache_hash
20c9366 
This fixes GH issue python-attrs#613 and python-attrs#494. It turns out that the hash
cache-clearing implementation for non-slots classes was flawed and never
quite worked properly. This switches away from using __setstate__ and
instead adds a custom __reduce__ that removes the cached hash value from the
default serialized output.
pganssle added a commit to pganssle/attrs that referenced this issue Jan 22, 2020
@pganssle
Implement __reduce__ with cache_hash
d1fb529 
This fixes GH issue python-attrs#613 and python-attrs#494. It turns out that the hash
cache-clearing implementation for non-slots classes was flawed and never
quite worked properly. This switches away from using __setstate__ and
instead adds a custom __reduce__ that removes the cached hash value from the
default serialized output.
pganssle added a commit to pganssle/attrs that referenced this issue Jan 27, 2020
@pganssle @godlygeek
Use an self-clearing subclass to store hash cache
e72f653 
Rather than attempting to remove the hash cache from the object state on
deserialization or serialization, instead we store the hash cache in an
object that reduces to None, thus clearing itself when pickled or
copied.

This fixes GH python-attrs#494 and python-attrs#613.

Co-authored-by: Matt Wozniski <[email protected]>
@pganssle pganssle mentioned this issue Jan 27, 2020
Merged
4 tasks
pganssle added a commit to pganssle/attrs that referenced this issue Feb 5, 2020
@pganssle @godlygeek
Use an self-clearing subclass to store hash cache
4e7315e 
Rather than attempting to remove the hash cache from the object state on
deserialization or serialization, instead we store the hash cache in an
object that reduces to None, thus clearing itself when pickled or
copied.

This fixes GH python-attrs#494 and python-attrs#613.

Co-authored-by: Matt Wozniski <[email protected]>
pganssle added a commit to pganssle/attrs that referenced this issue Feb 10, 2020
@pganssle @godlygeek
Use an self-clearing subclass to store hash cache
145ac67 
Rather than attempting to remove the hash cache from the object state on
deserialization or serialization, instead we store the hash cache in an
object that reduces to None, thus clearing itself when pickled or
copied.

This fixes GH python-attrs#494 and python-attrs#613.

Co-authored-by: Matt Wozniski <[email protected]>
hynek pushed a commit that referenced this issue Feb 10, 2020
@pganssle @godlygeek
Use a wrapper class for cache hash to prevent cache serialization (#620)
d6a65fb 
* Use an self-clearing subclass to store hash cache

Rather than attempting to remove the hash cache from the object state on
deserialization or serialization, instead we store the hash cache in an
object that reduces to None, thus clearing itself when pickled or
copied.

This fixes GH #494 and #613.

Co-authored-by: Matt Wozniski <[email protected]>

* Add test for two-argument __reduce__

I couldn't think of any way to make a useful and meaningful class that
has no state and also has no custom __reduce__ method, so I went
minimalist with it.

* Improve test for hash clearing behavior.

Previously, there was some miniscule risk of hash collision, and also it
was relying on the implementation details of `pickle` (the assumption
that `hash()` is never called as part of `pickle.loads`).

* Add improved testing around cache_hash

* Update src/attr/_make.py

Co-Authored-By: Ryan Gabbard <[email protected]>

* Update comment in slots_setstate

Since the cached hash value is not actually serialized in __getstate__,
__setstate__ is not actually "clearing" it on deserialization - it's
initializing the value to None.

* Add changelog entry

* Remove changelog for #611

This change was overshadowed by a more fundamental change in #620.

Co-authored-by: Matt Wozniski <[email protected]>
Co-authored-by: Ryan Gabbard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
No milestone
3 participants
@wsanchez @pganssle @gabbard