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

fix: wrap BatchRequest InputStream with BufferedInputStream #1749

Merged
merged 7 commits into from Mar 18, 2021

Conversation

Neenu1995
Copy link
Contributor

@Neenu1995 Neenu1995 commented Mar 15, 2021

If reading byte-by-byte from a GZipInputStream is slow, then wrapping in a BufferedInputStream should minimize this effect by reading in larger, buffered chunks.

Added a test to ensure that parsing a large BatchResponse does not take an exorbitant amount of time.

Fixes #1573 ☕️

@google-cla google-cla bot added the cla: yes label Mar 15, 2021
@Neenu1995 Neenu1995 changed the title Test batch request branch chore: test BatchRequest with gzip encoding Mar 15, 2021
@chingor13 chingor13 marked this pull request as ready for review Mar 16, 2021
@chingor13 chingor13 requested a review from as a code owner Mar 16, 2021
@Neenu1995 Neenu1995 added the automerge label Mar 16, 2021
@chingor13 chingor13 removed the automerge label Mar 16, 2021
@chingor13 chingor13 changed the title chore: test BatchRequest with gzip encoding fix: wrap BatchRequest InputStream with BufferedInputStream Mar 16, 2021
@Neenu1995 Neenu1995 requested a review from lesv Mar 16, 2021
Copy link
Member

@stephaniewang526 stephaniewang526 left a comment

LGTM!

@Neenu1995 Neenu1995 added the automerge label Mar 18, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit 07346d8 into master Mar 18, 2021
13 checks passed
@gcf-merge-on-green gcf-merge-on-green bot deleted the test-batchRequest-branch branch Mar 18, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge label Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants