rigetti / rpcq

The RPC framework and message specification for @rigetti Quantum Cloud Services.
Apache License 2.0
76 stars 29 forks source link

msgpack 1.0 introduces breaking changes #118

Closed appleby closed 1 year ago

appleby commented 4 years ago

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 commented 4 years ago

tangentially-related pyquil issue:

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

appleby commented 4 years ago

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.

alexjuda commented 2 years ago

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

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

stylewarning commented 2 years ago

Ping @notmgsk @kalzoo

FaustinCarter commented 1 year ago

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.

kalzoo commented 1 year ago

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