feat: began retry configuration implementation #674
Conversation
google-cla
bot
added
the
cla: yes
|
It's exciting to see this work coming together 😄 |
|
@shaffeeullah I have opened the retry-request PR here: googleapis/retry-request#35 |
gcf-owl-bot
bot
removed
the
owlbot:run
There was a problem hiding this comment.
LGTM, only question is, is there any chance that existing customer applications would hit one of the code paths that throws?
For instance, did we direct customers to set config.autoRetry manually at any point, and storage will now be setting config.retryOptions.autoRetry automatically?
| config.autoRetry !== undefined && | ||
| config.retryOptions?.autoRetry !== undefined | ||
| ) { | ||
| throw new ApiError( |
There was a problem hiding this comment.
If I'm understanding this logic, we through if a customer sets both autoRetry and retryOptions.autoRetry?
Is there any risk that when this is deployed existing customer applications are somehow setting both these parameters?
There was a problem hiding this comment.
All applications using nodejs-common should not be using these fields because they did not exist prior to this PR. Storage handles this case directly https://github.com/googleapis/nodejs-storage/pull/1474/files
* feat: began retry configuration implementation (#674) * feat: began retry configuration implementation * 🦉 Updates from OwlBot * added semicolon * 🦉 Updates from OwlBot * removed log statement * refactored constants * updated retry-request and made relevant updates in this lib * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * feat: implemented retry function (#675) * feat: implemented retry function * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md * refactored retryableErrorFn into retry config * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md * refactored code so test stub works as expected * removed print * added test * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md * refactored function Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/master/packages/owl-bot/README.md * updated retry-request to 4.2.2 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Custom retry strategy behavior. Will merge all retry PRs into the shaffeeullah/storageRetryStrategy branch and then merge that branch into master at the end. This avoids one giant PR.
There is one test currently failing. I believe it is because the options that the
makeRequestfunction is pulling fromretry-requestare not the options that we're looking for. This change is pending theretry-requestlibrary update.