Skip to content
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

[jsonrpsee http client]: support tokio1 & tokio02. #263

Merged
merged 2 commits into from
Apr 13, 2021

Conversation

niklasad1
Copy link
Member

@niklasad1 niklasad1 commented Apr 12, 2021

Closes #262

As substrate and some of the Rust ecosystem hasn't updated to tokio 1 yet, let's make it configurable to support either tokio0.2 or tokio 1.

//cc @akru can you try this to ensure that it actually fixes your issue ^^

@niklasad1 niklasad1 requested a review from ascjones April 12, 2021 08:08
Cargo.toml Outdated

[dependencies]
log = "0.4.14"
thiserror = "1.0.24"
futures = "0.3.13"
jsonrpsee-types = "=0.2.0-alpha.3"
jsonrpsee-ws-client = "=0.2.0-alpha.3"
jsonrpsee-http-client = "=0.2.0-alpha.3"
jsonrpsee-http-client = { version = "=0.2.0-alpha.3", default-features = false, optional = true }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it optional, in case we use default-features = false when depending on subxt. Wouldn't that cause a compilation error?

Copy link
Member Author

@niklasad1 niklasad1 Apr 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right good catch, it doesn't need to be optional.

However, if tokio1 or tokio02 is not enabled on jsonrpsee-http-client it will be a compilation error anyway. So doesn't really make a difference in practice.

Thus, if users don't use the default features and not enabled exactly one the features the compilation error might be a little tricky to figure out.

We got this https://github.com/paritytech/jsonrpsee/blob/master/http-client/src/lib.rs#L23-#L27 for now hopefully this goes away very soon but probably not ^^

Copy link
Member

@ascjones ascjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@niklasad1 niklasad1 merged commit 02960a3 into master Apr 13, 2021
@niklasad1 niklasad1 deleted the na-http-client-config-tokio1-or-tokio02 branch April 13, 2021 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support build on tokio0.2
2 participants