Pickled functions incompatible between 0.4.0 and 0.4.1

[nikhaldi]

When trying to unpickle a function with the 0.4.1 release from last week that was pickled with 0.4.0 I get this exception:

$ pip install cloudpickle==0.4.0
Collecting cloudpickle==0.4.0
  Using cached cloudpickle-0.4.0-py2.py3-none-any.whl
Installing collected packages: cloudpickle
  Found existing installation: cloudpickle 0.4.1
    Uninstalling cloudpickle-0.4.1:
      Successfully uninstalled cloudpickle-0.4.1
Successfully installed cloudpickle-0.4.0

$ python
Python 2.7.12 (default, Nov 19 2016, 06:48:10) 
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import cloudpickle
>>> with open('pickled', 'wb') as p:
...    cloudpickle.dump(lambda x: x, p)
... 
>>> 

$ pip install cloudpickle==0.4.1
Collecting cloudpickle==0.4.1
  Using cached cloudpickle-0.4.1-py2.py3-none-any.whl
Installing collected packages: cloudpickle
  Found existing installation: cloudpickle 0.4.0
    Uninstalling cloudpickle-0.4.0:
      Successfully uninstalled cloudpickle-0.4.0
Successfully installed cloudpickle-0.4.1

$ python
Python 2.7.12 (default, Nov 19 2016, 06:48:10) 
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import cloudpickle
>>> with open('pickled', 'rb') as p:
...   cloudpickle.load(p)
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "/usr/lib/python2.7/pickle.py", line 1384, in load
    return Unpickler(file).load()
  File "/usr/lib/python2.7/pickle.py", line 864, in load
    dispatch[key](self)
  File "/usr/lib/python2.7/pickle.py", line 1139, in load_reduce
    value = func(*args)
TypeError: _fill_function() takes exactly 6 arguments (5 given)

Same incompatibility in the other direction.

I realize this is fixable by synchronizing the cloudpickle version across environments, but this is not necessarily trivial in a distributed setting and the pickle may have been persisted somewhere a while ago before the version bump.

So my question is more if this break in compatibility was intentional. I'm somewhat surprised that this would break in a minor release. Does cloudpickle have an official policy about which versions should be compatible? Would be good to know so we know how to hedge against it.

[rgbkrk]

/cc @pitrou @ssanderson

We would have expected to be compatible with the last release, especially for something we called a patch release. Looks like that's not the case. Thank you for taking the time to report the issue.

[pitrou]

This was probably introduced by #118. But given the slightly experimental and in-flux nature of cloudpickle, I'm not sure we can promise to keep binary compatibility between releases. @mariusvniekerk

[nikhaldi]

Understandable that backwards-compatibility can't always be maintained. Backwards-compatibility is still hugely important for any serialization protocol though. If there's any way to redo this change in a way that doesn't break compatibility, that would be a good target for a 0.4.2 release. ;)

More generally, it would be a good idea to come up with an internal protocol versioning scheme as soon as possible. Otherwise everyone will eventually end up designing their own versioning scheme around cloudpickle.

[ssanderson]

It looks like the breaking change in question is that we updated the signature of _ffill_function from

def _fill_function(func, globals, defaults, dict, closure_values):

to

def _fill_function(func, globals, defaults, dict, module, closure_values):

The somewhat-gross way to restore backwards compatibility with 0.4.0 would be to do something like:

def _fill_function(*args):
    if len(args) == 5:
        # Backwards compat for cloudpickle v0.4.0.
        updated_args = args[:-1] + (None, args[-1],)
        return _ffill_function_internal(*updated_args)
    else:
        return _ffill_function_internal(*updated_args)

def _fill_function_internal(func, globals, defaults, dict, module, closure_values):
    # Existing body of `_fill_function`.

The main cost of that change, I think, would be that users who don't care about backwards compat would be paying a small performance overhead on every deserialization.

[ssanderson]

Probably more important than the particular fix at hand here is deciding what cloudpickle's policy should be around versioning and support for loading of old pickled objects. I think most of the maintainers of cloudpickle use it primarily as a wire format for shipping objects around a cluster rather than as a persistent storage format.

[nikhaldi]

If it's a wire protocol that implies there's a server and a client. You're always able to update all your servers and clients at the same time? Backwards-compatibility matters a lot for that use case too. On Oct 30, 2017 7:02 PM, "Scott Sanderson" <notifications@github.com> wrote: Probably more important than the particular fix at hand here is deciding what cloudpickle's policy should be around versioning and support for loading of old pickled objects. I think most of the maintainers of cloudpickle use it primarily as a wire format for shipping objects around a cluster rather than as a persistent storage format. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#126 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACvVOyp43UrAOAroC6FvUjkz5o2F-M0pks5sxlVqgaJpZM4QLWLO> .

[rgbkrk]

I think most of the maintainers of cloudpickle use it primarily as a wire format for shipping objects around a cluster rather than as a persistent storage format.

I'm one of these people that only uses it as a wire format. For the sake of handling rolling deploys, I'd hope that we're able to handle the previous release, say within a weeklong period.

[rgbkrk]

@nikhaldi can you contribute a test that works between the last 2 versions + the dev copy, incorporating it into our test harness?

[pitrou]

@ssanderson, your proposed fix works if you serialize with 0.4.0 and deserialize with 0.4.1, not the other way around.

Note that if _fill_function had been changed to take the module argument last, you could simply make it module=None in the function signature...

[nikhaldi]

@rgbkrk I'll put together at least a test and a minimal fix today