chore: add new request idempotency header x-goog-gcs-idempotency-token #2027
Conversation
BenWhitehead
added
do not merge
product-auto-label
bot
added
size: l
BenWhitehead
force-pushed
the
req-ids
branch
3 times, most recently
from
879135e to
b90f446
Compare
BenWhitehead
removed
the
do not merge
Page bucket's token will be taken care of by the generator updates that will come in the near future.
tritone
left a comment
tritone
left a comment
There was a problem hiding this comment.
Overall looks great, couple small questions based on my weak understanding of some java stuff...
| } | ||
|
|
||
| @NonNull | ||
| static GrpcCallContext newCallContext() { |
There was a problem hiding this comment.
Does this need a comment or more descriptive name clarifying that it adds the idempotency header? I think it could be confusing otherwise.
There was a problem hiding this comment.
Since this method is attached to the Retrying class, I feel it carries the context with it that the GrpcCallContext returned from this method contains things related to retries. I think the idempotency token is an implementation detail of what goes into the context.
| // 1. start resumable session | ||
| // 2. PUT first 256KiB | ||
| // 3. PUT second 256KiB | ||
| // 4. Finalize session and put final 45B |
There was a problem hiding this comment.
Just figured I'd ask because I am not great at following some of this testing code -- does the test assert that each of these 4 gets its own request ID, and that all retries for each request have the same ID?
There was a problem hiding this comment.
Mostly correct, we are asserting there is a distinct token for each of the 4 requests that are sent, however we are not inducing any failures in these tests to verify tokens are maintained for the same request across multiple attempts. Instead we are waiting on googleapis/storage-testbench#486 for the retry enforcement piece.
2 participants
No description provided.