rpcq icon indicating copy to clipboard operation
rpcq copied to clipboard

msgpack 1.0 introduces breaking changes

Open appleby opened this issue 5 years ago • 4 comments

Upgrading to msgpack v1.0.0+ breaks deserialization (and possibly other stuff). At some point we'd like to upgrade to 1.0.0, so investigate why and fix anything that needs fixing.

The msgpack 1.0.0 release includes the following under the Major breaking changes in msgpack 1.0 > Unpacker heading, which seems like a likely culprit:

Default value of strict_map_key is changed to True to avoid hashdos. You need to pass strict_map_key=False if you have data which contain map keys which type is not bytes or str.

First reported in https://github.com/rigetti/pyquil/issues/1178

Here is a repro case distilled from the one in https://github.com/rigetti/pyquil/issues/1178:

from pyquil import Program, get_qc
from pyquil.gates import *
from rpcq.messages import RewriteArithmeticRequest

qc=get_qc('2q-qvm')
program = Program()
theta = program.declare('theta', memory_type='REAL')
program += RY(theta, 0)
nq_program=qc.compiler.quil_to_native_quil(program)
arithmetic_request = RewriteArithmeticRequest(quil=nq_program.out())
qc.compiler.client.call("rewrite_arithmetic", arithmetic_request)

Running the above in an environment with msgpack 1.0 installed produces the following traceback:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/anaconda3/envs/rigetti-msgpack-test/lib/python3.7/site-packages/rpcq/_client.py", line 184, in call
    reply = from_msgpack(raw_reply)
  File "/anaconda3/envs/rigetti-msgpack-test/lib/python3.7/site-packages/rpcq/_base.py", line 178, in from_msgpack
    max_bin_len=max_bin_len, max_str_len=max_str_len)
  File "msgpack/_unpacker.pyx", line 195, in msgpack._cmsgpack.unpackb
ValueError: ParameterAref is not allowed for map key

appleby avatar Feb 23 '20 19:02 appleby

tangentially-related pyquil issue:

https://github.com/rigetti/pyquil/issues/1153

appleby avatar Feb 23 '20 19:02 appleby

Note that in the specific case above, it's probably possible to restructure the RewriteArithmeticResponse sent by quilc's rewrite-arithmetic-handler to use strings rather than ParameterArefs as dict keys and to make the corresponding change to pyquil as well.

In general, care must be taken, since rpcq has multiple downstream consumers that will all need to be tested to make sure they are compatible with msgpack 1.0. Even in the case of RewriteArithmeticResponse, above, quilc's rewrite-arithmetic-handler is a public interface, so changing it would technically be a breaking change (even if pyquil is likely the only consumer). From that perspective, the backwards-compatible option is to pass strict_map_key=False without changing the RewriteArithmeticResponse message format.

appleby avatar Feb 24 '20 15:02 appleby

Hey! @appleby, any plans for supporting for msgpack 1+? Please note that this issue prevents some projects from using pyquil:

  • pyquil depends on rpcq
  • there are other libraries that depend on msgpack>=1.0
  • consequence: projects that depend on msgpack>=1.0 can't use pyquil, even if the dependence on msgpack is indirect

Here's an illustration of my situation:

               ┌─────────┐    ┌────────────┐
            ┌─►│3rd-party├───►│msgpack>=1.0├──────┐
  ┌───────┐ │  │library  │    └────────────┘      ▼
  │my     │─┘  └─────────┘                     conflict
  │project├─┐                                     ▲
  └───────┘ │   ┌──────┐      ┌───────────┐       │
            └──►│pyquil├─────►│msgpack<1.0├───────┘
                └──────┘      └───────────┘

The errors I'm getting from pip:

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
[3rd party project] requires msgpack<2.0.0,>=1.0.0, but you have msgpack 0.6.2 which is incompatible.

OS: macOS 11.6.1 Python: 3.7.12 Pip: 21.3.1

alexjuda avatar Dec 03 '21 12:12 alexjuda

Ping @notmgsk @kalzoo

stylewarning avatar Dec 04 '21 00:12 stylewarning

This is now a huge issue for anyone using conda-forge and wanting to run a Python version newer than 3.8 since msgpack-python 0.6.2 (the most recent pre-1.0 version) has not been built against any Python version greater than 3.8.

FaustinCarter avatar Jan 05 '23 18:01 FaustinCarter

Solved by https://github.com/rigetti/rpcq/pull/156 in v3.11.0

kalzoo avatar Mar 23 '23 23:03 kalzoo