-
Notifications
You must be signed in to change notification settings - Fork 510
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
DO NOT MERGE migrate codebase to jsonrpsee #534
DO NOT MERGE migrate codebase to jsonrpsee #534
Conversation
jsonrpsee is missing a feature needed to implement the ethereum rpc api, so we are stuck until it is added: paritytech/jsonrpsee#599 |
@librelois I think you should be unblocked now, the APIs differ a bit from the |
let fut = async move { | ||
stream | ||
.take_while(|item| { | ||
std::future::ready(sink.send(&item).map_or_else( |
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.
this might leak if the stream never terminates see paritytech/jsonrpsee#627
Thanks @niklasad1, I plan to resume this integration (taking into account your remarks), as soon as we have a |
https://github.com/paritytech/substrate/tree/jsonrpsee-polkadot-v0.9.16 jsonrpsee will not be merged into master until polkadot v0.9.19 |
Thank, I'll use this branch :) |
@niklasad1 the substrate branch jsonrpsee-polkadot-v0.9.16 not include the API to configure the subscription ID generation, so we still stuck for now. |
It should be, can't you use https://github.com/paritytech/substrate/blob/jsonrpsee-polkadot-v0.9.16/client/service/src/config.rs#L103 or am I missing something? |
I'm sorry, it's because I had checked the branch 10 days ago when you told me, I didn't expect it to be updated in the meantime. |
Ok, for frontier it's good, but for moonbeam we also need a branch like |
I guess that you need to update all substrate and polkadot deps in another branch for moonbeam along with some glue code but can't you do this yourself or please elaborate why it has to be in the polkadot repo? |
All parachains based on polkadot v0.916 need such a branch to test jsonrpsee. You had made one for v0.9.13: https://github.com/paritytech/polkadot/tree/jsonrpsee-0.9.13 |
Sorry my bad there is already such branch: https://github.com/paritytech/polkadot/tree/jsonrpsee-v0.9.16 EDIT: I misunderstood I thought you meant something else :) |
This looks staled. I'm going to close it. Feel free to open a new PR when it's ready. |
Yes too many things have changed since then, I started over on a new branch, the PR is here: #695 It's not ready yet but I'm working on it, I hope to finish tomorrow. |
Draft migration from jsonrpc to jsonrpsee.