Yelp / bravado-core

Other
109 stars 98 forks source link

Pickling support #372

Closed macisamuele closed 4 years ago

macisamuele commented 4 years ago

The goal of this PR is to fix #370

In order to make bravado_core.spec.Spec pickable I had to define __getstate__ and __setstate__ methods into Spec and Resource class.

The reason of this is that pickling an object usually gets all the object attributes and tries to pickle them:

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.01%) to 99.175% when pulling c8cbd68a40bf886d0d31b645d8b830917766cd93 on macisamuele:maci-issue-370-pickling-support into ebaaae52319669ef75be54cada8c48671c0877e5 on Yelp:master.

macisamuele commented 4 years ago

@sjaensch for a couple of weeks I'll be far from a keyboard in order to make progress on this.

Definitely the point that you're raising is correct and that's the reason why I won't push more on introducing a similar feature until we don't have a clear idea on how to handle it.

I'll let you know when I get enough time to break the implementation and try to identify more weak points.

sjaensch commented 4 years ago

Looks good to me, but I see mypy and a test failure in the travis build. Could you take a look?

macisamuele commented 4 years ago

I was able to reproduce, locally, the test failure on Python 3.7, I'm going to update the test to fix the failure.

~The mypy failure is surprising to me as locally all looks good.~ ~I'll try to re-create a venv (with exactly the same dependencies) to try to reproduce the issue,~ The mypy failure is caused by the fact that the branch is not based on the latest master. Merging master into the branch makes the mypy reporting reproducible and so addressable.

sjaensch commented 4 years ago

This has been included in bravado-core 5.17.0.