fix: retry uploads only conditionally #316
Conversation
google-cla
bot
added
the
cla: yes
andrewsg
changed the title
product-auto-label
bot
added
the
api: storage
andrewsg
force-pushed
the
retry-resumable
branch
from
5a8763f to
c535039
Compare
andrewsg
force-pushed
the
retry-resumable
branch
from
c535039 to
ccf37a1
Compare
| @@ -2058,6 +2060,15 @@ def _do_upload( | |||
| **only** response in the multipart case and it will be the | |||
| **final** response in the resumable case. | |||
| """ | |||
| if if_metageneration_match is None and num_retries is None: | |||
There was a problem hiding this comment.
I think we should update the docstring as well to make it clear that setting num_retries can be used to override the idempotency requirement (assuming that's the intention), to make sure that users know the implications.
There was a problem hiding this comment.
Yes, that's the intention. Will do.
|
|
||
| # Adjust num_retries expectations to reflect the conditional default in | ||
| # _do_upload() | ||
| if num_retries is None and if_metageneration_match is None: |
There was a problem hiding this comment.
Can you comment on to what extent the conditional is covered by test cases?
There was a problem hiding this comment.
Ah yeah that's right, good point.
There was a problem hiding this comment.
Branch coverage of the code I've added is 100% according to the coverage tool. We are exercising scenarios where if_metageneration_match = true and num_retries is not None.
andrewsg
added
the
automerge
|
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
gcf-merge-on-green
bot
removed
the
automerge
* fix: retry uploads only conditionally * update docstring for num_retries
* fix: retry uploads only conditionally * update docstring for num_retries
No description provided.