Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add batch request support #99

Merged
merged 3 commits into from
May 4, 2023
Merged

Conversation

tarassh
Copy link
Contributor

@tarassh tarassh commented May 2, 2023

This pull request adds support for batched requests in JSON-RPC v2 as described in the JSON-RPC 2.0 specification.

Changes:

  • Add a check in a new handleReader function to handle batched requests.
  • Change in rpcError function response status code to 200. JSON-RPC errors are part of response body.
  • Add new unit tests to cover batched request processing.

Motivation:

Batched requests allow clients to send multiple requests in a single HTTP request, reducing overhead and allowing for more efficient communication. This is particularly useful in situations where multiple requests need to be made in a short period of time, such as when syncing data in real time.

Testing:

New unit tests have been added to cover the processing of batched requests. Additionally, manual testing has been performed to ensure that the implementation correctly handles a variety of batched request scenarios, including valid and invalid inputs.

Please provide any feedback or suggestions for improvement.

@tarassh tarassh changed the title Add batch support feat: add batch request support May 2, 2023
@tarassh tarassh marked this pull request as ready for review May 2, 2023 19:51
@raulk raulk requested a review from Stebalien May 3, 2023 18:55
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

One question/comment. One nit on handling whitespace. Otherwise LGTM. Thanks!

* Add proper status codes to error responses
* Add more unit tests
@tarassh tarassh requested a review from Stebalien May 4, 2023 08:09
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

❤️

Stebalien added a commit to filecoin-project/lotus that referenced this pull request May 4, 2023
@Stebalien
Copy link
Member

I'll merge this once it passes CI in lotus.

@tarassh
Copy link
Contributor Author

tarassh commented May 4, 2023

Awesome, thank you.

@raulk
Copy link
Member

raulk commented May 4, 2023

@Stebalien we'll probably want to get this deployed to Glif and other hosted providers to unblock ngram and others!

@Stebalien Stebalien merged commit f007863 into filecoin-project:master May 4, 2023
@tarassh
Copy link
Contributor Author

tarassh commented May 5, 2023

I see PR didn't pass warning checks, should I create a new PR to fix it?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants