Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow subclasses to override the deserialization logic #102

Closed
wants to merge 2 commits into from

Conversation

bukzor
Copy link

@bukzor bukzor commented Oct 3, 2012

This matches #96

I've added a unit test which demonstrates how this can be used to transform unpickling errors to cache misses. This is implemented by refactoring the _PylibMC_Unpickle function to be a method: Client.deserialize.

I imagine the next logical step is to do the same refactorization for _PylibMC_Pickle / serialize, but the project is in a fully usable state as-is, and this change closes our issue. Allowing overrides to both serialize/deserialize will close a couple of your other issues: "Custom Pickler implementations", "It should be possible to specify pickle protocol version".

@@ -394,7 +394,7 @@ static PyObject *_PylibMC_parse_memcached_value(char *value, size_t size,

switch (dtype) {
case PYLIBMC_FLAG_PICKLE:
retval = _PylibMC_Unpickle(value, size);
retval = PyObject_CallMethod((PyObject *)self, "deserialize", "s#", value, size);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you extend the callback method to pass in the flags too? This would allow PHP memcached (and other) implementations to inspect the memcached flags for deserialization.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how.

Feel free to fork / edit / pull request.

@lericson
Copy link
Owner

Very nice work! I am however skeptical of a couple of things that I think we should at least discuss before going further:

  • Documentation in the docs/ directory
  • What is CacheMiss? What does it do, and why isn't it just a MemcachedError subclass? Is it really a CacheMiss when the serialization layer fails? Shouldn't that be somehow taken care of?
  • What are the performance implications? Can you run runbench.py before and after this patch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants