-
Notifications
You must be signed in to change notification settings - Fork 98
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
Smart read retries #27
Smart read retries #27
Conversation
src/table.js
Outdated
]); | ||
|
||
stream.on('error', error => { | ||
// TODO - DO NOT MERGE - @stephenplusplus is this unpipe needed? |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/table.js
Outdated
if (stream._ended) { | ||
return next(); | ||
|
||
const retryStream = through.obj(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/table.js
Outdated
const requestStream = this.requestStream(grpcOpts, reqOpts); | ||
requestStream.on('request', () => numRequestsMade++); | ||
|
||
const stream = pumpify.obj([ |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/table.js
Outdated
]); | ||
|
||
stream.on('error', error => { | ||
// TODO - DO NOT MERGE - @stephenplusplus is this unpipe needed? |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/table.js
Outdated
-1 | ||
); | ||
}; | ||
const greaterThan = (lhs, rhs) => { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/table.js
Outdated
const greaterThan = (lhs, rhs) => { | ||
return lessThan(rhs, lhs); | ||
}; | ||
const greaterThanOrEqualTo = (lhs, rhs) => { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/table.js
Outdated
const normalizeRangeValue = rangeSide => { | ||
return is.object(rangeSide) ? rangeSide.value : rangeSide; | ||
}; | ||
const startValue = normalizeRangeValue(range.start); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #27 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 9 9
Lines 949 1000 +51
=====================================
+ Hits 949 1000 +51
Continue to review full report at Codecov.
|
src/table.js
Outdated
const RETRY_STATUS_CODES = new Set([4, 10, 14]); | ||
const MUTATE_ROWS_RETRYABLE_STATUS_CODES = new Set([4, 10, 14]); | ||
|
||
// TODO - DO NOT MERGE - Figure out exactly what codes need to be here. Some combination of |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
23bee22
to
8264f95
Compare
@stephenplusplus Were there any other concerns or comments you had? If not I think we're good to merge |
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
Fixes #7
Appropriate docs were updated (if necessary)