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

Various Subscription improvements #177

Merged
merged 13 commits into from
Feb 5, 2024
Merged

Conversation

prestwich
Copy link
Member

@prestwich prestwich commented Feb 1, 2024

Motivation

Closes #170

Solution

  • Add a configuration option to PubSubFrontend that allows setting the default subscription channel size limit
  • Allow requests makers to specify that a request is a subscription
  • Improve service handling of subs

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@prestwich prestwich added the enhancement New feature or request label Feb 1, 2024
@prestwich prestwich self-assigned this Feb 1, 2024
@prestwich prestwich force-pushed the prestwich/sub-buffer-size-config branch from 5d2f9ed to f7b6b0d Compare February 1, 2024 23:12
crates/json-rpc/src/request.rs Outdated Show resolved Hide resolved
crates/json-rpc/src/request.rs Outdated Show resolved Hide resolved
@prestwich prestwich changed the title Allow connector to configure subscription buffer size Allow frontend to configure subscription buffer size Feb 2, 2024
@prestwich prestwich changed the title Allow frontend to configure subscription buffer size Various Subscription improvements Feb 2, 2024
crates/json-rpc/src/request.rs Outdated Show resolved Hide resolved
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this solution gives the user the most control and we're no longer limited to eth_subscribe

Comment on lines +28 to +32
/// Indicates that the request is a non-standard subscription (i.e. not
/// "eth_subscribe").
pub fn set_is_subscription(&mut self) {
self.is_subscription = true;
}
Copy link
Member

Choose a reason for hiding this comment

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

this could use some helper functions to:
set sub state with bool arg

Comment on lines +64 to +67
/// "eth_subscribe").
pub fn set_is_subscription(&mut self) {
self.meta.set_is_subscription()
}
Copy link
Member

Choose a reason for hiding this comment

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

same

Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

LG to the ME

@prestwich prestwich merged commit 32618e9 into main Feb 5, 2024
14 checks passed
@prestwich prestwich deleted the prestwich/sub-buffer-size-config branch February 5, 2024 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants