-
Notifications
You must be signed in to change notification settings - Fork 231
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
rpc: Add structured querying for event subscriptions #584
Conversation
This commit adds structured querying to RPC event subscription, which replaces the `String`-based interface for subscription. No query parsing is implemented yet. Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
This merges `master` into `rpc/query` while also introducing a locking mechanism in the tests such that no two integration tests that perform transaction subscriptions to a live Tendermint node can run at the same time. Without this locking mechanism, Tokio executes the two tests simultaneously, which causes non-deterministic results. Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
Signed-off-by: Thane Thomson <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #584 +/- ##
========================================
Coverage ? 41.8%
========================================
Files ? 164
Lines ? 11704
Branches ? 2649
========================================
Hits ? 4898
Misses ? 6438
Partials ? 368
Continue to review full report at Codecov.
|
@thanethomson Great stuff! I took the liberty pushing a few non-controversial changes (imho):
The last commit (27fe514) merges the |
Looks like
|
Excellent, thanks @romac! Looks great - I like the simplification 👍 Are there any clippy lints we can enable that can warn us about there being more idiomatic ways to implement specific pieces of code? |
Not to my knowledge. Closest lint I could find is https://rust-lang.github.io/rust-clippy/master/#inherent_to_string but it would not have helped in this case. I guess there are valid use cases for implementing ToString or defining a From instance directly which is why Clippy does not warn about that, or perhaps those lints are just currently missing. |
My best guess for the I can try to repro it locally and see if there's a quick fix to work around the issue. This might also be worth reporting upstream. Edit: reproduced the issue locally and confirmed that an explicit deref fixed the issue. I can push up a fix (which might make the behavior a bit more explicit/straightforward in general) |
Plausible cause of the nightly regression: rust-lang/rust#77638 |
Partially addresses #582.
This PR adds structured querying to RPC event subscription, which replaces the
String
-based interface for subscription.No query parsing is implemented yet, as it's not clear yet whether we need it at the moment and we need to figure out how to translate the query PEG from the Go version of Tendermint.