Revert `66fb6a1`: "Avoid compatibility problems with PyPy + dill" #109

Peque · 2016-03-21T12:41:26Z

Dill was recently added as a new serialization method in Pyro. However, some ugly fixes where made to workaround PyPy+dill compatibility issues.

These fixes should be reverted as soon as Dill fixes its PyPy compatibility issues. It seems there is a patch already that may fix those issues, but it has not being integrated in Dill yet.

I am subscribed to both threads in Dill, so I will update this issue with any relevant information.

irmen · 2016-04-25T21:23:27Z

Note that commit aa7a715 extends these workarounds to add IronPython as well. It doesn't like dill either.
This means we'll now probably get conflicts if we simply want to revert the pypy workaround. Not a big deal, it's not much code

Peque · 2016-04-26T08:15:53Z

@irmen Thanks. Fortunately you are right, it is not a lot of code, so it will be easy to revert anyway. I just wrote a message to Dill's developer to know if there is any work-in-progress or update in relation to PyPy compatibility.

mmckerns · 2016-05-05T17:59:31Z

The patch noted above is merged, and I've also fixed a number of the remaining compatibility issues. There are still a few that I can see that remain, however, which I need to get back to soon.

I should note (I saw discussion in the original PR from @Peque) that you can import dill and then stop dill from overriding pickle -- that way, you can override things in a more piecemeal fashion if you like.

>>> import dill
>>> dill.extend(False)

irmen · 2016-06-27T17:40:48Z

any progress with dill? (sorry I haven't bothered to investigate myself).

Peque · 2016-06-27T18:21:05Z

@irmen yes, there has been some progress! I will try to test the latest changes this week (did not find time before). 😇

irmen · 2016-06-27T22:58:54Z

awesome, thanks.

Peque · 2016-06-30T12:50:17Z

It seems the tests do not pass yet. 7 fails, detailed here:

https://gist.github.com/Peque/be752e1fa6f4e845ace9607d803fb0f7

They all end in the form:

[...]
  File "/usr/lib64/pypy-4.0.1/lib-python/2.7/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/media/src/Pyro4/.tox/pypy/site-packages/dill/dill.py", line 1015, in save_wrapper_descriptor
    pickler.save_reduce(_getattr, (obj.__objclass__, obj.__name__,
AttributeError: 'function' object has no attribute '__objclass__'

@mmckerns Any ideas? 😊

mmckerns · 2016-06-30T14:00:10Z

Aha… this appears to be an issue that is not tested in the main dill test suite, but is tested in dill_bugs.py which is a collection of known failure cases. Apparently dill is failing in pypy in an unexpected way, and this case is replicating the errors you are seeing. Let's move this to a new ticket (uqfoundation/dill#175) in dill for this particular case.

Peque · 2016-06-30T14:38:30Z

@mmckerns Thanks. ❤️

I will keep an eye on that issue and try again when it is solved.

irmen · 2016-07-04T19:44:18Z

Ok. I don't think I'll wait for this for the upcoming Pyro release, it will (hopefully) land in the one after that.

Peque · 2016-07-07T08:00:47Z

@irmen Sure, go ahead. Let us hope it will be ready for the next release. 😊

irmen · 2016-12-13T22:32:07Z

any progress on this front? 👷

Peque · 2016-12-14T20:06:37Z

@irmen I don't think so. I may try to contact Mike this weekend and propose him a deal: he works on this, I help him setting up CI in dill with pytest and Travis (I've done some initial coding and seems doable)... Don't know if he'll be interested, though. 😅

irmen · 2016-12-14T20:31:48Z

okay thanks for the update!

mmckerns · 2016-12-20T21:59:06Z

I have not had time to resolve this issue yet, but you can track the progress here: (uqfoundation/dill#175). I may be able to get to it over the next week or so, and hopefully it's easy to solve. Basically, I think the only compatibility issue I can see is with the wrapper_descriptor.

Peque · 2017-01-16T18:38:20Z

It seems there are some updates related to this:

uqfoundation/dill#175

There is a discussion in PyPy's Bitbucket (where Armin Rigo himself has been very diligent and helpful):

https://bitbucket.org/pypy/pypy/issues/2464/getset_descriptor-cannot-access

So, hopefully, everything will be fixed and ready for the next PyPy and dill versions. 😊

Thanks a lot, @mmckerns.

irmen · 2017-01-16T20:23:32Z

Nice to hear!

mmckerns · 2017-01-17T16:17:41Z

The latest builds of pypy and dill pass all tests that I have for pypy + dill compatibility. You guys are probably better suited to further test this than I am. Please report any issues you might find.

irmen · 2017-01-17T17:58:53Z

thanks
it does require a new pypy version from what I understand (which one?).
I'm not sure yet if this is reason enough to keep the checks in place but make them know about the minimum pypy version required to make things work, or just assume it will be resolved externally.

Peque · 2017-01-17T18:56:54Z

@irmen Last commit in PyPy related to this issue is from an hour ago, so it will require next version (not-yet-tagged). I guess right now the only way to try this is with the PyPy latest development version. I haven't download it nor try it yet, though.

Maybe next week I'll find some time to download it and try it to see if everything is fixed. Regarding the "keep the checks in place" issue... I guess that is as you wish, you are the owner. 😉

mmckerns · 2017-01-17T19:16:23Z

I have been testing against the pypy nightly builds, which are pretty easy to install and work with.

irmen · 2017-01-18T23:09:38Z

I suppose it is enough to add a warning in the docs that using dill will require a very recent pypy version with the needed changes to support it. And remove the workarounds from the code. That way we don't have to wait for an official release version and don't have to stick around with ugly version checks in the pyro code. For Ironpython it just doesn't work but I guess the choice of using a particular python implementation that doesn't fully support the serializer the user chooses, is the problem of the user and not of Pyro.

Peque · 2017-01-25T18:30:42Z

@irmen Agree. Although maybe we should wait for an official dill and pypy releases. Otherwise Pyro4 tests will fail in Travis (unless we configure the pypy tests to be executed with a nightly build or something...). If there is no need to hurry this changes, I would rather wait. 😉

PS: couldn't try the latest changes with pypy nightly builds (dependencies on libraries not available in my system: Fedora). I guess I could try with a containerized Ubuntu, but have not found the time yet.

mmckerns · 2017-01-25T19:03:19Z

A new dill release is imminent. Should be out in a few days.

irmen · 2017-01-25T20:26:57Z

There's no need to hurry things. Although I don't know what pypy's release schedule is

mmckerns · 2017-01-25T21:11:37Z

No worries, it's a planned release.

Peque · 2017-01-26T08:30:55Z

@irmen They do not have a fixed schedule, I think, but it is usually each couple of months. This year there is no release yet, but last year there were in November, September, August, June, May, April, March... So I hope it wont take much longer to see a new release this year. 😊

irmen · 2017-01-27T22:32:43Z

commit c6cf81c removes most of the platform checks with dill, in anticipation of the new releases

Peque · 2017-01-29T20:39:03Z

@irmen You may want to change .travis config file meanwhile (#145).

irmen · 2017-02-05T13:59:14Z

I noticed the new dill release came out recently, and indeed it fixes the issues with pypy. For Pyro's test suite, we don't even need the latest pypy release to successfully complete

Peque mentioned this issue Jun 30, 2016

compatibility with pypy uqfoundation/dill#73

Closed

irmen closed this as completed Feb 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert `66fb6a1`: "Avoid compatibility problems with PyPy + dill" #109

Revert `66fb6a1`: "Avoid compatibility problems with PyPy + dill" #109

Peque commented Mar 21, 2016

irmen commented Apr 25, 2016

Peque commented Apr 26, 2016

mmckerns commented May 5, 2016

irmen commented Jun 27, 2016

Peque commented Jun 27, 2016

irmen commented Jun 27, 2016

Peque commented Jun 30, 2016

mmckerns commented Jun 30, 2016 •

edited

Loading

Peque commented Jun 30, 2016

irmen commented Jul 4, 2016

Peque commented Jul 7, 2016

irmen commented Dec 13, 2016

Peque commented Dec 14, 2016

irmen commented Dec 14, 2016

mmckerns commented Dec 20, 2016

Peque commented Jan 16, 2017

irmen commented Jan 16, 2017

mmckerns commented Jan 17, 2017

irmen commented Jan 17, 2017

Peque commented Jan 17, 2017

mmckerns commented Jan 17, 2017

irmen commented Jan 18, 2017 •

edited

Loading

Peque commented Jan 25, 2017

mmckerns commented Jan 25, 2017

irmen commented Jan 25, 2017

mmckerns commented Jan 25, 2017

Peque commented Jan 26, 2017

irmen commented Jan 27, 2017

Peque commented Jan 29, 2017

irmen commented Feb 5, 2017

Revert 66fb6a1: "Avoid compatibility problems with PyPy + dill" #109

Revert 66fb6a1: "Avoid compatibility problems with PyPy + dill" #109

Comments

Peque commented Mar 21, 2016

irmen commented Apr 25, 2016

Peque commented Apr 26, 2016

mmckerns commented May 5, 2016

irmen commented Jun 27, 2016

Peque commented Jun 27, 2016

irmen commented Jun 27, 2016

Peque commented Jun 30, 2016

mmckerns commented Jun 30, 2016 • edited Loading

Peque commented Jun 30, 2016

irmen commented Jul 4, 2016

Peque commented Jul 7, 2016

irmen commented Dec 13, 2016

Peque commented Dec 14, 2016

irmen commented Dec 14, 2016

mmckerns commented Dec 20, 2016

Peque commented Jan 16, 2017

irmen commented Jan 16, 2017

mmckerns commented Jan 17, 2017

irmen commented Jan 17, 2017

Peque commented Jan 17, 2017

mmckerns commented Jan 17, 2017

irmen commented Jan 18, 2017 • edited Loading

Peque commented Jan 25, 2017

mmckerns commented Jan 25, 2017

irmen commented Jan 25, 2017

mmckerns commented Jan 25, 2017

Peque commented Jan 26, 2017

irmen commented Jan 27, 2017

Peque commented Jan 29, 2017

irmen commented Feb 5, 2017

Revert `66fb6a1`: "Avoid compatibility problems with PyPy + dill" #109

Revert `66fb6a1`: "Avoid compatibility problems with PyPy + dill" #109

mmckerns commented Jun 30, 2016 •

edited

Loading

irmen commented Jan 18, 2017 •

edited

Loading