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

Respect timeout on client.read_rows. Don't resume on DEADLINE_EXCEEDED errors. #8025

Merged

Conversation

tswast
Copy link
Contributor

@tswast tswast commented May 17, 2019

DEADLINE_EXCEEDED should not be resumed. Currently, you can set a
timeout on reads, but the reader will reconnect after the timeout value
finishes.

Always resuming the stream masked internal issue 132959978 from clients,
where the default deadline is much too low for read streams.

Wait for the deadline configuration fix for read rows to go in before
merging.

…EDED` errors.

DEADLINE_EXCEEDED should not be resumed. Currently, you can set a
timeout on reads, but the reader will reconnect after the timeout value
finishes.

Always resuming the stream masked internal issue 132959978 from clients,
where the default deadline is much too low for read streams.

Wait for the deadline configuration fix for read rows to go in before
merging.
@tswast tswast force-pushed the b132959978-bqstorage-deadline-exceeded branch from 114b096 to a8e38b1 Compare May 20, 2019 15:38
@tswast tswast marked this pull request as ready for review May 20, 2019 15:38
@tswast tswast requested a review from a team May 20, 2019 15:38
Copy link

@kmjung kmjung left a comment

Choose a reason for hiding this comment

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

LGTM.

@tswast tswast merged commit b498978 into googleapis:master May 20, 2019
@tswast tswast deleted the b132959978-bqstorage-deadline-exceeded branch May 20, 2019 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants