-
Notifications
You must be signed in to change notification settings - Fork 46
Support msgpack 1.0.0 #156
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
Conversation
…sgpack 1.0 makes the driver unable to parse server response
In CI:
It seems we should support both old and new implementations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, squash commits and explain changes in the commit message (just like you did for the PR description). Don't forget to include Fixes #155
or Closes #155
tag.
See also the comment below.
if msgpack.version >= (1, 0, 0): | ||
unpacker = msgpack.Unpacker(use_list=True, raw=False, strict_map_key=False) | ||
else: | ||
unpacker = msgpack.Unpacker(use_list=True, raw=False) | ||
elif conn.encoding is not None: | ||
unpacker = msgpack.Unpacker(use_list=True, encoding=conn.encoding) | ||
if msgpack.version >= (1, 0, 0): | ||
unpacker = msgpack.Unpacker(use_list=True, encoding=conn.encoding, strict_map_key=False) | ||
else: | ||
unpacker = msgpack.Unpacker(use_list=True, encoding=conn.encoding) | ||
else: | ||
unpacker = msgpack.Unpacker(use_list=True) | ||
if msgpack.version >= (1, 0, 0): | ||
unpacker = msgpack.Unpacker(use_list=True, strict_map_key=False) | ||
else: | ||
unpacker = msgpack.Unpacker(use_list=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can reduce code duplication by unpacking a dictionary to keyword parameters. A kind of this (not tested):
kwargs = dict(use_list=True)
if msgpack.version >= (1, 0, 0):
# XXX: Explain why it is necessary.
kwargs['strict_map_key'] = False
if msgpack.version >= (0, 5, 2) and conn.encoding == 'utf-8':
# Get rid of the following warning.
# > PendingDeprecationWarning: encoding is deprecated,
# > Use raw=False instead.
kwargs['raw'] = False
elif conn.encoding is not None:
kwargs['encoding'] = conn.encoding
unpacker = msgpack.Unpacker(**kwargs)
From python-msgpack 1.0.0 release notes:
Are'we affected by this? |
In fact, no:
|
Closed in favor of PR #173. |
In msgpack 1.0.0, the default value of strict_map_key is changed to True to avoid hashdos. However, it will break the Tarantool driver, causing an exception:
As a result, the driver is unable to parse server response. The issue is reported at #155