Use only the highest, premium quality pickler available at runtime.

[akatrevorjay]

Also include a forewarning note on Python "pickleableness".

This also allows easier changing as it's now a constant. While not perfect, it's certainly helpful for me at least!

The issue with version 0 is that some objects which should be pickleable are not.

Namely this scenario:

• An object defines __getstate__ to select the state to be pickled and restored.
• This object contains a non-pickleable slotted object in an attribute value (ie defines __slots__)
• Said attribute happens to be specifically excluded from the state dictionary returned from __getstate__ of it's parent, but alias, cPython with pickle version zero returns:

*** TypeError: a class that defines __slots__ without defining __getstate__ cannot be pickled

Fun stuff. Thanks for the lib.

[jogo]

Why not use https://docs.python.org/2/library/pickle.html#pickle.HIGHEST_PROTOCOL?

Also note that in python3 the default is not 0. https://docs.python.org/3.1/library/pickle.html

[akatrevorjay]

Because -1 is always the highest protocol.

[akatrevorjay]

I can use the const if you want, but using -1 is pretty standard imo and doesn't have an attr lookup tax (which is of course not worth mentioning though)

[akatrevorjay]

Correct, in python 3 the default is the new py3+ pickle. But in pymemcache, we always use zero and it unfortunately is a pretty broken version :)

[jogo]

@cgordon looks like you added this originally any specific reason for the old behavior? Looks like this is based on https://github.com/linsomniac/python-memcached/blob/5b75728565393d53768f4622fdd43e3e742c4741/memcache.py

Also this is a a backwards incompatible change.

Perhaps it would be better to make this field an argument that can be set?

[nichochar]

LGTM, I don't have a strong opinion about -1 vs HIGHEST_PROTOCOL, and this seems like an improvement.

Thanks for the change @akatrevorjay

[cgordon]

@jogo Yes, the "python_memcache_[de]serializer" functions are meant to emulate the default behavior of the python-memcache library. If this breaks that functionality, it is going to hurt all the people (including Pinterest) that rely on it. If -1 and 0 are compatible then this change is fine.

[akatrevorjay]

You can always load any supported version by your runtime, so this is fully backwards compatible unless you say migrate to python 3 only on a few nodes, in which case you'll need to plan for such an event anyway, and at least it's easily runtime patchable. I'll submit another patch next week that be makes this into a class for this very reason of better runtime configuration configurability.

On Fri, Jun 2, 2017, 1:31 PM Charles Gordon ***@***.***> wrote: @jogo <https://github.com/jogo> Yes, the "python_memcache_[de]serializer" functions are meant to emulate the default behavior of the python-memcache library. If this breaks that functionality, it is going to hurt all the people (including Pinterest) that rely on it. If -1 and 0 are compatible then this change is fine. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#156 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQGIl1yIwcYQlNXrB56QoT9lxW2W33Kks5sAHEAgaJpZM4Nsad3> .

-- Ty, Trevor

[akatrevorjay]

Of course, you could be using some ancient python as well on some nodes, but that would have to be so ancient that it would be irresponsible to run them imo. Thoughts?

On Fri, Jun 2, 2017, 1:35 PM Trevor Joynson ***@***.***> wrote: You can always load any supported version by your runtime, so this is fully backwards compatible unless you say migrate to python 3 only on a few nodes, in which case you'll need to plan for such an event anyway, and at least it's easily runtime patchable. I'll submit another patch next week that be makes this into a class for this very reason of better runtime configuration configurability. On Fri, Jun 2, 2017, 1:31 PM Charles Gordon ***@***.***> wrote: > @jogo <https://github.com/jogo> Yes, the > "python_memcache_[de]serializer" functions are meant to emulate the default > behavior of the python-memcache library. If this breaks that functionality, > it is going to hurt all the people (including Pinterest) that rely on it. > If -1 and 0 are compatible then this change is fine. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#156 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABQGIl1yIwcYQlNXrB56QoT9lxW2W33Kks5sAHEAgaJpZM4Nsad3> > . > -- Ty, Trevor

-- Ty, Trevor