feat(gax): add ResumableUploadClient.startUpload() and supporting types - #14138
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
whowes
changed the title
whowes
changed the title
c0e396a to
3f2b223
Compare
|
/gemini review |
3f2b223 to
dd5cb3e
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces low-level resumable upload components to the GAX library, including the ResumableUploadClient interface, the ResumableUploadSession metadata class, and the StartUploadRequest configuration class, along with their corresponding unit tests. The reviewer's feedback suggests a minor readability and performance improvement in StartUploadRequest.Builder to assign the result of getPath() to a local variable instead of invoking the getter multiple times.
whowes
changed the title
dd5cb3e to
4758549
Compare
4758549 to
0229e07
Compare
0229e07 to
fad9e9c
Compare
083072d to
6878a77
Compare
6878a77 to
83f8c5b
Compare
|
|
||
| /** Returns the optional initial JSON request payload. */ | ||
| @Nullable | ||
| public abstract String getJsonPayload(); |
There was a problem hiding this comment.
Is this for the proto message (e.g. CreateVideoRequest) defined in the RPC?
There was a problem hiding this comment.
Yes - expanded the comment to clarify this.
There was a problem hiding this comment.
Is it possible to use a proto type instead of String? Or when we made it here, the proto message has already been serialized?
There was a problem hiding this comment.
Thanks for pushing on this - I do think it makes sense for this interface to accept/return protos rather than JSON, and have serialization/deserialization happen in the impl.
While implementing that (in a subsequent PR) it became pretty clear that the other concerns in this request POJO (path and query params) are retrievable from the ApiMethodDescriptor associated with the resumable upload-powered method (along with the request formatter and response parser). So if the client impl is created with a reference to that descriptor (similarly to long running clients), there's actually no reason to have a separate container—just passing the request proto is sufficient.
83f8c5b to
55ce02f
Compare
96a4d11 to
cd7189e
Compare
a362b5f to
7199b9d
Compare
04b2e13 to
854ebb2
Compare
adb6a7d to
efc1a40
Compare
efc1a40 to
f1e5a2a
Compare
f1e5a2a to
df3845d
Compare
df3845d to
251a1f0
Compare
251a1f0 to
8b0416c
Compare
8b0416c to
b034273
Compare
|
|
2 participants
HTTP/JSON client implementation for startUpload is in #14139.