CP-34942: compatibility with rpclib 7 and 8

xapi-project / xcp-idl

Interface descriptions for services running on an XCP host.

Other

9 stars 41 forks source link

CP-34942: compatibility with rpclib 7 and 8 #314

Closed psafont closed 3 years ago

psafont commented 4 years ago

There were two breaking changes affecting the tests:

Avoiding marshalling integer as i8 requires the addition of the strict flag.
A new function for json-rpc to have calls without responses got added

To test the changes I recommend to

check out the xs-opam PR locally
create a new opam switch and activate it
add the local xs-opam repo to the switch with opam repo add xs-opam-local location/to/xs-opam/folder
check out this PR and run opam install --deps-only . and make test

Depends on https://github.com/xapi-project/xs-opam/pull/511 and https://github.com/xapi-project/message-switch/pull/56

psafont commented 4 years ago

Are the tests the only thing that need to be fixing, or do we need to update all users of xcp-idl to have the additional parameters? (e.g. xapi-storage, etc.)

Only the marshaller had to be modified in this case, strict will probably need to be set explicitly when the default changes, not sure when.

Clients need to be modified to account for the new base64 type of values, however. (like the idl in xen-api)

psafont commented 3 years ago

To do that upgrade the PR in rpclib needs to be merged, tagged and released. I would not want to carry the change in a patchqueue.

In the end 4 repositories are affected, the upgrade could be done as part of the angstrom upgrade (which breaks 2 repos). I don't think it's a big burden if the change is not immediately integrated and can be easily managed.

mseri commented 3 years ago

FWIW I reserved time to do that next week, also releasing a ppx_deriving_rpc that drops base and supports the latest ppxlib (I will release it for both 7.2 and 8.0)