feat(generator): generate resumable upload client methods - #9285
Conversation
Adds the `resumable_upload_methods` generator parameter: a semicolon-separated list of `Service.Method` pairs. Selected methods are augmented with a resumable upload descriptor, removed from the simple methods list, and exposed on the service as `resumableUploads`, which the templates use to generate the resumable upload clients. Plumbs the parameter through the CLI, the bazel rule and the naming options, and refreshes the pubsub api dump baselines for the new `resumableUploads` field.
Templates for the cjs and esm flavors now turn methods selected with `resumable_upload_methods` into resumable upload methods: - expose a ResumableUploadDescriptor for each selected method and wire the gax resumableUploadStub into the client's inner API calls - keep the methods out of the service stub used for unary/paging calls - add the getResumableSource() helper and the session-returning method, which require an HTTP(S) transport and reject gRPC channel credentials - generate sample snippets for resumable upload methods Adds a synthetic resumable.proto fixture, the resumable-upload and resumable-upload-esm baseline tests, and baselines generated with the current templates.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for generating resumable upload methods in the GAPIC generator for TypeScript. It introduces a new resumable_upload_methods configuration option, updates the generator schema and templates (for both CommonJS and ESM formats) to generate the necessary client methods and sample code, and adds corresponding unit and baseline tests. The review feedback is highly constructive, pointing out an unsafe non-null assertion on the service name that should use a safe fallback, and identifying redundant empty conditional blocks in the sample templates that cause unnecessary blank lines.
| bc => bc.serviceName === parameters.service.name, | ||
| ); | ||
| const resumableUploadMethods = resumableUploadMethodNames( | ||
| parameters.service.name!, |
There was a problem hiding this comment.
Using the non-null assertion operator ! on parameters.service.name is unsafe because parameters.service.name can be undefined or null in the AST/descriptor. To prevent potential runtime errors or unexpected behavior, use a safe fallback like parameters.service.name || ''.
parameters.service.name || '',| {% if method.resumableUpload %} | ||
| {% endif %} |
There was a problem hiding this comment.
This empty {% if %} block is redundant and generates unnecessary blank lines in the output. It should be removed.
| {% if method.resumableUpload %} | ||
| {% endif %} |
There was a problem hiding this comment.
This empty {% if %} block is redundant and generates unnecessary blank lines in the output. It should be removed.
`{% if method.resumableUpload %}` had an empty body, so its only effect
was an extra newline: resumable upload samples rendered two blank lines
after the require, where every other generated sample has one. Remove
the block and regenerate the affected create_resumable_upload baselines
for both the cjs and esm variants.
Found by the Gemini review on googleapis#9285.
1 participant
Templates for the cjs and esm flavors now turn methods selected with
resumable_upload_methodsinto resumable upload methods. This depends on the generator parameter PR in this series (merge that one first).ResumableUploadDescriptorfor each selected method and wire the gaxresumableUploadStubinto the client's inner API callsgetResumableSource()and the session-returning method, which require an HTTP(S) transport and reject gRPC channel credentialsresumable.protofixture and theresumable-upload/resumable-upload-esmbaseline testsVerification
npm run compilenpm test— 192 passing, including the two new baselinesGenerated output for existing APIs is unchanged: every template addition is guarded on the service having resumable upload methods.
Merges after: #9284
Related to: #9283