Skip to content

New python-msgpack version brokes connector #155

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.

Sign up for GitHub

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

Jump to bottom
Closed
RepentantGopher opened this issue Mar 12, 2020 · 4 comments · Fixed by #173
Closed

New python-msgpack version brokes connector #155

RepentantGopher opened this issue Mar 12, 2020 · 4 comments · Fixed by #173
Assignees
@Totktonada
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@RepentantGopher
Copy link

New version of python-msgpack library was released which enforce only string keys in map by default.
This change brokes whole connector because version of python msgpack library is not enforced in requirements.

Commit msgpack/msgpack-python@d8e3cf0

Error on connection to tarantool:

test_tags.py:6: in <module>
    from .common import (
common.py:22: in <module>
    operational_storage = tarantool.connect("localhost", 3303, "tarantool", "tarantool")
venv/lib/python3.7/site-packages/tarantool/__init__.py:51: in connect
    encoding=encoding)
venv/lib/python3.7/site-packages/tarantool/connection.py:130: in init
    self.connect()
venv/lib/python3.7/site-packages/tarantool/connection.py:208: in connect
    raise NetworkError(e)
E   tarantool.error.NetworkError: int is not allowed for map key
@pcdinh
Copy link

pcdinh commented Mar 12, 2020 •
edited
Loading

I got the same issue in Ubuntu 18.04, Python 3.8.2, Tarantool 0.6.6

@pcdinh pcdinh mentioned this issue Mar 12, 2020
Closed
@Totktonada
Copy link
Member

Should be fixed with PR #156.

@Totktonada Totktonada added the bug Something isn't working label Jul 8, 2020
@Totktonada Totktonada linked a pull request Jul 10, 2020 that will close this issue
Support msgpack 1.0.0 #156
Closed
@Totktonada Totktonada added the good first issue Good for newcomers label Aug 18, 2020
artembo added a commit that referenced this issue Aug 24, 2020
@artembo
msgpack: Add 1.0.0 version support
fa34c9f 
Fixes #155
@artembo artembo linked a pull request Aug 24, 2020 that will close this issue
msgpack: Add 1.0.0 version support #167
Closed
@artembo artembo mentioned this issue Aug 24, 2020
Closed
artembo added a commit that referenced this issue Aug 24, 2020
@artembo
msgpack: Add 1.0.0 version support
4fe621e 
Since msgpack 1.0.0 was released Unpacker does not have
strict_map_key param and it is True by default now.

It makes the Connector fail with an error:
`ValueError: int is not allowed for map key`

To solve this issue the __init__ method of the Response
object was refactored and the strict_map_key param is
added for versions less than 1.0.0

Fixes #155
@artembo
Copy link
Contributor

artembo commented Aug 26, 2020

In the new release of msgpack also encoding parameter was removed.

Remove encoding option from the Packer and Unpacker.

@Totktonada suggested

If the encoding option is given to the connector, then you can programmatically go through recursively and do .decode (encoding = 'utf-8') for all bytes (Python 3) or str (Python 2) values. It seems like this should work, and fully transparently support everything that was previously supported:

Python 2:

>>> import msgpack
>>> msgpack.unpackb(b'\xd9\x08\xd0\xb0\xd0\xb1\xd0\xb2\xd0\xb3', raw=True).decode(encoding='utf-8')
u'\u0430\u0431\u0432\u0433'
>>> msgpack.unpackb(b'\xd9\x08\xd0\xb0\xd0\xb1\xd0\xb2\xd0\xb3', encoding='utf-8')
u'\u0430\u0431\u0432\u0433'

Python 3:

>>> import msgpack
>>> msgpack.unpackb(b'\xd9\x08\xd0\xb0\xd0\xb1\xd0\xb2\xd0\xb3', raw=True).decode(encoding='utf-8')
'абвг'
>>> msgpack.unpackb(b'\xd9\x08\xd0\xb0\xd0\xb1\xd0\xb2\xd0\xb3', encoding='utf-8')
__main__:1: DeprecationWarning: encoding is deprecated, Use raw=False instead.
'абвг'

@Totktonada
Copy link
Member

I dived a bit further and it reminds me changes we did for msgpack-0.5.2.

We already use raw=False when encoding is 'utf-8' since msgpack-0.5.2 (2097884), because otherwise it emits the deprecation warning regarding encoding option usage. So we should just continue passing raw=False on msgpack-1.0.0.

Totktonada added a commit that referenced this issue Aug 26, 2020
@Totktonada
refactoring: extract msgpack unpacker tuning code
c502a30 
We will need a bit more logic in unpacker configuration code to support
msgpack-1.0.0, so it is convenient to factor out it from actual creation
of the unpacker instance.

No behaviour is changed here.

Part of #155
Totktonada added a commit that referenced this issue Aug 26, 2020
@Totktonada
Support msgpack-1.0.0 (unpacking part)
15ae91d 
msgpack-1.0.0 changes default value of 'raw' and 'strict_map_key'
options. We should handle different versions of the library and provide
the same behaviour across them.

This version of the msgpack library also drops support of 'encoding'
option. We already use raw=True/False msgpack option to implement
encoding=None/'utf-8' connector option, so the only change we should do
about this is to explicitly forbid other encoding values. Unlikely using
of text encodings other than UTF-8 is popular.

Part of #155
Totktonada added a commit that referenced this issue Aug 26, 2020
@Totktonada
refactoring: wrap msgpack.dumps() into own method
fd751e9 
We'll need to tune msgpack options in order to support msgpack-1.0.0 and
it is convenient to do so in one place.

Aside of this, reusing of an msgpack.Packer() instance looks good for
performance matters, however I guess that an actual difference is
negligible.

No behaviour is changed.

Part of #155
Totktonada added a commit that referenced this issue Aug 26, 2020
@Totktonada
Support msgpack-1.0.0 (packing part)
6aea93c 
The default value of the 'use_bin_type' option was changed in
msgpack-1.0.0. We should support different versions of the library and
provide the same behaviour across them.

Aside of this, 'encoding' option was dropped since this version of the
msgpack library, but we didn't actually support it for packing.  Binary
strings are packed as is, Unicode strings are encoded as UTF-8 and
nothing is changed regarding this.

Fixes #155
@Totktonada Totktonada mentioned this issue Aug 26, 2020
Merged
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
refactoring: extract msgpack unpacker tuning code
6761fa5 
We will need a bit more logic in unpacker configuration code to support
msgpack-1.0.0, so it is convenient to factor out it from actual creation
of the unpacker instance.

No behaviour is changed here.

Part of #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
Support msgpack-1.0.0 (unpacking part)
72bc88f 
msgpack-1.0.0 changes default value of 'raw' and 'strict_map_key'
options. We should handle different versions of the library and provide
the same behaviour across them.

This version of the msgpack library also drops support of 'encoding'
option. We already use raw=True/False msgpack option to implement
encoding=None/'utf-8' connector option, so the only change we should do
about this is to explicitly forbid other encoding values. Unlikely using
of text encodings other than UTF-8 is popular.

Part of #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
refactoring: wrap msgpack.dumps() into own method
1166622 
We'll need to tune msgpack options in order to support msgpack-1.0.0 and
it is convenient to do so in one place.

Aside of this, reusing of an msgpack.Packer() instance looks good for
performance matters, however I guess that an actual difference is
negligible.

No behaviour is changed.

Part of #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
Support msgpack-1.0.0 (packing part)
98d2574 
The default value of the 'use_bin_type' option was changed in
msgpack-1.0.0. We should support different versions of the library and
provide the same behaviour across them.

Aside of this, 'encoding' option was dropped since this version of the
msgpack library, but we didn't actually support it for packing. Binary
strings are packed as is (as mp_str), Unicode strings are encoded as
UTF-8 (as mp_str too) and nothing is changed regarding this.

Fixes #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
travis-ci: verify different msgpack library versions
ce6d483 
Also bump msgpack PyPI package name in requirements.txt. See the note
about the library name change in [1]:

 | Package name on PyPI was changed to msgpack from 0.5.

[1]: https://github.com/msgpack/msgpack-python/blob/8fb709f2e0438862020d8810fa70a81fb5dac7d4/README.md

Follows up #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
travis-ci: verify different msgpack library versions
190e1c8 
Also bump msgpack PyPI package name in requirements.txt. See the note
about the library name change in [1]:

 | Package name on PyPI was changed to msgpack from 0.5.

[1]: https://github.com/msgpack/msgpack-python/blob/8fb709f2e0438862020d8810fa70a81fb5dac7d4/README.md

Follows up #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
refactoring: extract msgpack unpacker tuning code
847327b 
We will need a bit more logic in unpacker configuration code to support
msgpack-1.0.0, so it is convenient to factor out it from actual creation
of the unpacker instance.

No behaviour is changed here.

Part of #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
Support msgpack-1.0.0 (unpacking part)
30d54f0 
msgpack-1.0.0 changes default value of 'raw' and 'strict_map_key'
options. We should handle different versions of the library and provide
the same behaviour across them.

This version of the msgpack library also drops support of 'encoding'
option. We already use raw=True/False msgpack option to implement
encoding=None/'utf-8' connector option, so the only change we should do
about this is to explicitly forbid other encoding values. Unlikely using
of text encodings other than UTF-8 is popular.

Part of #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
refactoring: wrap msgpack.dumps() into own method
02998f5 
We'll need to tune msgpack options in order to support msgpack-1.0.0 and
it is convenient to do so in one place.

Aside of this, reusing of an msgpack.Packer() instance looks good for
performance matters, however I guess that an actual difference is
negligible.

No behaviour is changed.

Part of #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
Support msgpack-1.0.0 (packing part)
45d530c 
The default value of the 'use_bin_type' option was changed in
msgpack-1.0.0. We should support different versions of the library and
provide the same behaviour across them.

Aside of this, 'encoding' option was dropped since this version of the
msgpack library, but we didn't actually support it for packing. Binary
strings are packed as is (as mp_str), Unicode strings are encoded as
UTF-8 (as mp_str too) and nothing is changed regarding this.

Fixes #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
Eliminate deprecation warning re use_bin_type
542ba75 
When 'use_bin_type' option is not set explicitly, msgpack-0.5.0 (and
only it, not 0.5.1 or newer) warns a user that its default value will be
changed in a future.

The main reason of the change is to allow
test_03_discovery_bad_good_addresses() test from test_mesh.py to pass on
msgpack-0.5.0: it counts deprecation warnings produced by a call and so
affected if unexpected warnings are emitted.

This commit is prerequisite to enable msgpack-0.5.0 in CI, which is done
in the next commit.

Follows up #155
Totktonada added a commit that referenced this issue Aug 28, 2020
@Totktonada
travis-ci: verify different msgpack library versions
7c9d9cb 
Also bump msgpack PyPI package name in requirements.txt. See the note
about the library name change in [1]:

 | Package name on PyPI was changed to msgpack from 0.5.

[1]: https://github.com/msgpack/msgpack-python/blob/8fb709f2e0438862020d8810fa70a81fb5dac7d4/README.md

Follows up #155
Totktonada added a commit that referenced this issue Aug 29, 2020
@Totktonada
travis-ci: verify different msgpack library versions
a02ad89 
Also bump msgpack PyPI package name in requirements.txt. See the note
about the library name change in [1]:

 | Package name on PyPI was changed to msgpack from 0.5.

[1]: https://github.com/msgpack/msgpack-python/blob/8fb709f2e0438862020d8810fa70a81fb5dac7d4/README.md

Follows up #155
Totktonada added a commit that referenced this issue Aug 29, 2020
@Totktonada
travis-ci: verify different msgpack library versions
053aa88 
Also bump msgpack PyPI package name in requirements.txt. See the note
about the library name change in [1]:

 | Package name on PyPI was changed to msgpack from 0.5.

I choose versions for testing this way: we have 0.4.0 in
requirements.txt, so it is the baseline. We should test at least 0.4.0,
0.5.0, 0.6.0 and 1.0.0. I also added 0.6.2, because this version is
installed on my Gentoo box and because it is the last release before
1.0.0.

[1]: https://github.com/msgpack/msgpack-python/blob/8fb709f2e0438862020d8810fa70a81fb5dac7d4/README.md

Follows up #155
Totktonada added a commit that referenced this issue Aug 29, 2020
@Totktonada
refactoring: extract msgpack unpacker tuning code
024298b 
We will need a bit more logic in unpacker configuration code to support
msgpack-1.0.0, so it is convenient to factor out it from actual creation
of the unpacker instance.

No behaviour is changed here.

Part of #155
Totktonada added a commit that referenced this issue Aug 29, 2020
@Totktonada
Support msgpack-1.0.0 (unpacking part)
bf640ec 
msgpack-1.0.0 changes default value of 'raw' and 'strict_map_key'
options. We should handle different versions of the library and provide
the same behaviour across them.

This version of the msgpack library also drops support of 'encoding'
option. We already use raw=True/False msgpack option to implement
encoding=None/'utf-8' connector option, so the only change we should do
about this is to explicitly forbid other encoding values. Unlikely using
of text encodings other than UTF-8 is popular.

Part of #155
Totktonada added a commit that referenced this issue Aug 29, 2020
@Totktonada
refactoring: wrap msgpack.dumps() into own method
47c3448 
We'll need to tune msgpack options in order to support msgpack-1.0.0 and
it is convenient to do so in one place.

Aside of this, reusing of an msgpack.Packer() instance looks good for
performance matters, however I guess that an actual difference is
negligible.

No behaviour is changed.

Part of #155
Totktonada added a commit that referenced this issue Aug 29, 2020
@Totktonada
Support msgpack-1.0.0 (packing part)
81d5253 
The default value of the 'use_bin_type' option was changed in
msgpack-1.0.0. We should support different versions of the library and
provide the same behaviour across them.

Aside of this, 'encoding' option was dropped since this version of the
msgpack library, but we didn't actually support it for packing. Binary
strings are packed as is (as mp_str), Unicode strings are encoded as
UTF-8 (as mp_str too) and nothing is changed regarding this.

Fixes #155
Totktonada added a commit that referenced this issue Aug 29, 2020
@Totktonada
Eliminate deprecation warning re use_bin_type
17de9a5 
When 'use_bin_type' option is not set explicitly, msgpack-0.5.0 (and
only it, not 0.5.1 or newer) warns a user that its default value will be
changed in a future.

The main reason of the change is to allow
test_03_discovery_bad_good_addresses() test from test_mesh.py to pass on
msgpack-0.5.0: it counts deprecation warnings produced by a call and so
affected if unexpected warnings are emitted.

This commit is prerequisite to enable msgpack-0.5.0 in CI, which is done
in the next commit.

Follows up #155
Totktonada added a commit that referenced this issue Aug 29, 2020
@Totktonada
travis-ci: verify different msgpack library versions
148d12e 
Also bump msgpack PyPI package name in requirements.txt. See the note
about the library name change in [1]:

 | Package name on PyPI was changed to msgpack from 0.5.

I choose versions for testing this way: we have 0.4.0 in
requirements.txt, so it is the baseline. We should test at least 0.4.0,
0.5.0, 0.6.0 and 1.0.0. I also added 0.6.2, because this version is
installed on my Gentoo box and because it is the last release before
1.0.0.

[1]: https://github.com/msgpack/msgpack-python/blob/8fb709f2e0438862020d8810fa70a81fb5dac7d4/README.md

Follows up #155
@Totktonada Totktonada self-assigned this Aug 29, 2020
Totktonada added a commit that referenced this issue Dec 28, 2020
@Totktonada
pypi: correct msgpack library name
7d82c57 
In short, the msgpack library changes its name and now offered as
'msgpack' rather than 'msgpack-python'. All new versions of the library
are shipped under the new name, so we should change our dependency
information.

See commit 148d12e ('travis-ci: verify
different msgpack library versions') for details.

Follows up #155
@Totktonada Totktonada mentioned this issue Dec 28, 2020
Merged
Totktonada added a commit that referenced this issue Dec 28, 2020
@Totktonada
pypi: correct msgpack library name
6210545 
In short, the msgpack library changes its name and now offered as
'msgpack' rather than 'msgpack-python'. All new versions of the library
are shipped under the new name, so we should change our dependency
information.

See commit 148d12e ('travis-ci: verify
different msgpack library versions') for details.

Follows up #155
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Totktonada Totktonada

Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
4 participants
@pcdinh @Totktonada @artembo @RepentantGopher