Conversation
5a8763f to
c535039
Compare
c535039 to
ccf37a1
Compare
| **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.
|
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. |
* fix: retry uploads only conditionally * update docstring for num_retries
* fix: retry uploads only conditionally * update docstring for num_retries
No description provided.