Python 3 serialization fixes, serialization improvements #131
Conversation
| if isinstance(value, str): | ||
| # Check against exact types so that subclasses of native types will be | ||
| # restored as their native type | ||
| if value_type == six.binary_type: |
There was a problem hiding this comment.
(nits) I think value_type is bytes is more readable.
There was a problem hiding this comment.
Hi,
Thanks for fixing the tests (by making them run). Also thanks for the python3 enhancements. We don't use python3 at Pinterest so it's a little hard for us to catch when things go wrong, always a pleasure when the community patches stuff.
There are errors in the automatic test build, you can click on it under the PR comments view, or click here directly.
Those need to be fixed before we can approve.
| pass | ||
|
|
||
|
|
||
| @pytest.mark.unit() |
There was a problem hiding this comment.
This is the manual call to make sure it's called? Thanks for adding!
| client.flush_all() | ||
|
|
||
| check(b'byte string') | ||
| check(u'unicode string') |
There was a problem hiding this comment.
Can we also check try some harder cases:
'olé'(with and without specifying theuunicode prefix- list of strings with unicode in them
There was a problem hiding this comment.
I've added in defaultdict (a subclass of dict) to test pickle comparisons as well - as it's available in Python 2.5+.
| check({'a': 'pickle'}) | ||
|
|
||
|
|
||
| @pytest.mark.integration() |
| def test_str(self): | ||
| self.check('value') | ||
| def test_bytes(self): | ||
| self.check(b'value') |
There was a problem hiding this comment.
let's try this with unicode characters.
Also maybe explicitely ask for FLAG_BYTES as it should default to this?
There was a problem hiding this comment.
FLAG_BYTES didn't exist before - so I've added that in, even though it's 0.
|
Not too sure what to do about the pypy3 failure. The Travis build environment has changed between 15th-16th November. A newer version of virtualenv is installed, which is trying to install a newer version of pip - which now only supports Python 3.3+ for Python 3. The Travis environment is calling pypy3 v2.4.0 - a Python 3.2 implementation of Python. I could just disable it, unless there's another option you prefer? |
|
Thanks for raising this, I have opened a ticket in order to drop pypy3. |
|
After exploring this issue, we have decided to drop support for that interpreter version. Feel free to rebase and push, and we can go ahead with this PR. |
Currently this will fail with unicode strings on Python 3, as the pymemcache client will return a byte string - but the deserializer doesn't change it back to a unicode string.
The pymemcache client will return a byte string, so we'll do the same to test that the deserializer works as expected. This currently fails with Python 3.
- Add text serializer/deserializer - Now more strict with serializing a type
More pythonic
Works across all versions of Python, no need to use six for it
- A string (will vary on Python version) - Another unicode string - A list (pickled) - A defaultdict (pickled)
Use some common characters
Unused in serde itself (it's a default), but used for testing comparisons
tomkins
force-pushed
the
serde-serialization
branch
from
dd46704 to
164f4cf
Compare
|
Rebased and pushed. |
|
Thanks for the improvements @tomkins tomkins, we really appreciate them! |
I was looking at trying to use/resurrect django-pymemcache, however in trying to update it and get tests working on it - pymemcache serialization seems to have some issues with byte/unicode strings. This is mostly a Python 3 problem, but could cause problems elsewhere.
To solve this problem, I've taken a similar approach to https://github.com/linsomniac/python-memcached/pull/100/files:
Also
test_serde.pywasn't being picked up with pytest as it didn't have a decorator - so I've added that.