lericson / pylibmc

A Python wrapper around the libmemcached interface from TangentOrg.
http://sendapatch.se/projects/pylibmc/
BSD 3-Clause "New" or "Revised" License
479 stars 137 forks source link

Discrepancy between pylibmc and python-memcached in storing bools #245

Open hyperair opened 5 years ago

hyperair commented 5 years ago

While comparing python-memcached and pylibmc behaviours, I found that python-memcached was able to store and retrieve bool values as bools, while pylibmc would store them as integers being 1 or 0. It looks like python-memcached pickles bools but pylibmc stores them as integers.

lericson commented 5 years ago

Did it cause errors?

On 12 Nov 2018, at 10:29, Chow Loong Jin notifications@github.com wrote:

While comparing python-memcached and pylibmc behaviours, I found that python-memcached was able to store and retrieve bool values as bools, while pylibmc would store them as integers being 1 or 0. It looks like python-memcached pickles bools but pylibmc stores them as integers.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

hyperair commented 5 years ago

On Mon, Nov 12, 2018 at 02:58:27PM +0000, Ludvig Ericson wrote:

Did it cause errors?

Well, storing and retrieving a bool using pylibmc converts bools into ints, but doing the same in python-memcached returns a bool. The latter behaviour seems more correct.

-- Kind regards, Loong Jin

lericson commented 5 years ago

I see. Maybe we should reintroduce bool storage but as int-with-extra flag. Going through the pickle machinery for something so simple seems... counterproductive, if you see my reasoning.

On 14 Nov 2018, at 19:33, Chow Loong Jin notifications@github.com wrote:

On Mon, Nov 12, 2018 at 02:58:27PM +0000, Ludvig Ericson wrote:

Did it cause errors?

Well, storing and retrieving a bool using pylibmc converts bools into ints, but doing the same in python-memcached returns a bool. The latter behaviour seems more correct.

-- Kind regards, Loong Jin — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

hyperair commented 5 years ago

On Wed, Nov 14, 2018 at 12:54:47PM -0800, Ludvig Ericson wrote:

I see. Maybe we should reintroduce bool storage but as int-with-extra flag. Going through the pickle machinery for something so simple seems... counterproductive, if you see my reasoning.

Yes, I see your reasoning. Perhaps you could reintroduce FLAG_BOOL as 1 << 31 to hopefully avoid conflicts with any new flags python-memcached introduces in the future.

-- Kind regards, Loong Jin

sergeief commented 3 years ago

We've encountered this after switching from python-memcached. In our case, uncached value returns as True and cached as 1, which caused a bug in the code. Is there workaround available?

hyperair commented 3 years ago

You can subclass pylibmc.Client and override the serialize method to match python-memcached's behaviour

import pickle
import pylibmc

class Client(pylibmc.Client):
    def serialize(self, value):
        """
        Overridden serialize method. See pylibmc.client.Client.serialize for
        more info
        """

        # Ensure that bools are pickled to match python-memcached behaviour.
        # Remove this when https://github.com/lericson/pylibmc/issues/245 is
        # fixed.
        if isinstance(value, bool):
            return pickle.dumps(value), 1
        else:
            return super().serialize(value)