feat(gax): add ResumableUploadClient.startUpload() and supporting types - #14138
feat(gax): add ResumableUploadClient.startUpload() and supporting types#14138whowes wants to merge 1 commit into
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
|
|
| * alignment or granularity requirements apply. | ||
| * @return a new {@link ResumableUploadSession} instance | ||
| */ | ||
| public static ResumableUploadSession create(String uploadUrl, long chunkGranularity) { |
There was a problem hiding this comment.
Can we use a builder patten instead of the create helper?
|
|
||
| public StartUploadRequest build() { | ||
| String path = getPath(); | ||
| if (path != null && path.startsWith("/")) { |
There was a problem hiding this comment.
Do we need this check? I think path would be something we set so we know the exact format?
| } | ||
|
|
||
| Map<String, List<String>> params = getQueryParams(); | ||
| if (params != null && !params.isEmpty()) { |
There was a problem hiding this comment.
I think this map is also something we pass in so we don't have to copy it again?
|
|
||
| /** 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?
2 participants
HTTP/JSON client implementation for startUpload is in #14139.