-
Notifications
You must be signed in to change notification settings - Fork 553
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
feat: add a configurable maximum batch size for RPC requests #2939
Conversation
I still need to find a way to implement some unit tests for the batch requests |
@melekes, do you think that it would be helpful if I implemented some refactoring for this handler cometbft/rpc/jsonrpc/server/http_server.go Line 263 in ea1f896
I thought of combining this into just one middleware. I think doing this as a |
sound good to me 👍 |
…/cometbft into andy/2867-rpc-batch-size-config
This should be ready to review @melekes, implemented a test too. I've tagged it to be back ported to |
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.
Thanks @andynog ❤️
Makes sense to backport to 0.38 IMO. |
If you want to test this locally you can:
|
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.
👍
close: #2867 Adds middleware for the JSONRPC server to enforce a configurable maximum batch size for RPC requests. --- #### PR checklist - [ ] Tests written/updated - [X] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [X] Updated relevant documentation (`docs/` or `spec/`) and code comments - [X] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec (cherry picked from commit aa1caba)
close: #2867 Adds middleware for the JSONRPC server to enforce a configurable maximum batch size for RPC requests. --- #### PR checklist - [ ] Tests written/updated - [X] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [X] Updated relevant documentation (`docs/` or `spec/`) and code comments - [X] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec (cherry picked from commit aa1caba) # Conflicts: # docs/references/config/config.toml.md
…#2939) (#2980) close: #2867 Adds middleware for the JSONRPC server to enforce a configurable maximum batch size for RPC requests. --- #### PR checklist - [ ] Tests written/updated - [X] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [X] Updated relevant documentation (`docs/` or `spec/`) and code comments - [X] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec <hr>This is an automatic backport of pull request #2939 done by [Mergify](https://mergify.com). --------- Co-authored-by: Andy Nogueira <me@andynogueira.dev> Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
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.
Thank you for this. :)
I wish we could simplify the code under the node
package, moving it to the rpc
package (where it belongs), but lets keep this to a next refactoring effort.
close: #2867
Adds middleware for the JSONRPC server to enforce a configurable maximum batch size for RPC requests.
PR checklist
.changelog
(we use unclog to manage our changelog)docs/
orspec/
) and code comments