Skip to content

test(gax): add showcase harness for resumable uploads - #9288

Draft
feywind wants to merge 3 commits into
googleapis:mainfrom
feywind:resumable/gax-showcase
Draft

test(gax): add showcase harness for resumable uploads#9288
feywind wants to merge 3 commits into
googleapis:mainfrom
feywind:resumable/gax-showcase

Conversation

@feywind

@feywind feywind commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Adds an end-to-end harness for the gapic-showcase ResumableUploadService, so the resumable upload client path can be exercised against a real server. It uses the resumable upload support from the google-gax PR in this series, so merge that one first.

  • checked-in generated client for google.showcase.v1beta1.ResumableUploadService
  • sample.js, which uploads a local file through a resumable session
  • run.sh, which downloads gapic-showcase, compiles the client against the local google-gax checkout and runs the sample
  • harness README plus a pointer from test/README.md, and a .gitignore entry for the generated protos

This is test tooling only; nothing in the published package changes.

Merge first: #9287
Related to: #9283

Add the client-side implementation of the resumable upload protocol:

- ResumableUploadDescriptor and ResumableUploadSession, plus the
  resumableUploadStub that generated clients wire into createApiCall
- resumableSourceFromFile, a seekable source backed by a local file
- CallOptions.resumableUpload carrying the transport context that
  generated clients pass to the stub
- exports from index, fallback and descriptor, and client-libraries docs
- unit and hermetic system tests covering the state machine, transient
  retries, recovery from state mismatches and resume from a saved URL

The onProgress callback may return void; the documented usage logs
progress without returning a value.
Adds an end-to-end harness for the gapic-showcase ResumableUploadService:
a checked-in generated client, a sample that uploads a local file through
a resumable session, and a run.sh that downloads the showcase server,
compiles the client against the local google-gax checkout and runs the
sample. Also documents the harness in test/README.md and ignores the
protos it generates.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements client-side support for the resumable upload protocol in google-gax, introducing the ResumableUploadSession state machine, file-backed seekable sources, and a showcase test harness. Feedback on the changes highlights an incorrect throughput constant (DEFAULT_UPLOAD_RATE_BYTES_PER_MS) that scales to gigabytes instead of megabytes per millisecond, as well as documentation examples in client-libraries.md that incorrectly reference uploadStream instead of uploadSource.

export const DEFAULT_STALL_TIMEOUT_MS = 15 * 1000;
// Assumed sustained upload throughput, in bytes per millisecond, used to
// scale the global deadline when `uploadSize` is provided (~5 MB/s).
const DEFAULT_UPLOAD_RATE_BYTES_PER_MS = 5 * 1024 * 1024;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The value of DEFAULT_UPLOAD_RATE_BYTES_PER_MS seems to be incorrect based on the accompanying comment. The comment suggests an assumed throughput of ~5 MB/s, but the constant is defined as 5 * 1024 * 1024.

If this value is in bytes per millisecond as the variable name suggests, it equates to ~5 GB/s, which is likely far too high and will result in unreasonably short deadlines for large uploads.

For an assumed rate of 5 MB/s, the value in bytes per millisecond should be closer to 5 * 1024 * 1024 / 1000, which is approximately 5243. A value of 5 * 1024 (5120) would be a reasonable approximation.

Suggested change
const DEFAULT_UPLOAD_RATE_BYTES_PER_MS = 5 * 1024 * 1024;
const DEFAULT_UPLOAD_RATE_BYTES_PER_MS = 5 * 1024;

Comment on lines +162 to +170
const helper = await client.createResumableUpload(request);
await helper.start({
uploadStream: dataStream,
chunkSize: 8 * 1024 * 1024, // 8MB chunks
onProgress: status => {
console.log(`Committed ${status.bytesUploaded} bytes to ${status.uploadUrl}`);
},
});
const response = await helper.finished();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The example for starting a resumable upload appears to be incorrect. The start method expects an uploadSource parameter of type ResumableSource, but the example uses uploadStream. Additionally, the dataStream variable is not defined, which could be confusing.

Note: Since this is an auto-generated file, please do not edit it directly as these changes will be overwritten. Instead, apply this fix upstream in the generator or templates.

Suggested change
const helper = await client.createResumableUpload(request);
await helper.start({
uploadStream: dataStream,
chunkSize: 8 * 1024 * 1024, // 8MB chunks
onProgress: status => {
console.log(`Committed ${status.bytesUploaded} bytes to ${status.uploadUrl}`);
},
});
const response = await helper.finished();
const uploadSource = client.getResumableSource(filePath);
const helper = await client.createResumableUpload(request);
await helper.start({
uploadSource,
chunkSize: 8 * 1024 * 1024, // 8MB chunks
onProgress: status => {
console.log('Committed ' + status.bytesUploaded + ' bytes to ' + status.uploadUrl);
},
});
const response = await helper.finished();
References
  1. Do not manually edit auto-generated files to fix typos or make other changes, as these edits will be overwritten during the next regeneration. Instead, apply the fixes upstream in the generator or templates.

Comment on lines +179 to +184
const helper = await client.createResumableUpload();
await helper.start({
uploadStream: dataStream,
resumeUrl: savedUploadUrl,
});
const response = await helper.finished();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the previous example, this code snippet for resuming an upload uses the incorrect uploadStream parameter. It should be uploadSource. When resuming, you still need to provide the same payload source.

Note: Since this is an auto-generated file, please do not edit it directly as these changes will be overwritten. Instead, apply this fix upstream in the generator or templates.

Suggested change
const helper = await client.createResumableUpload();
await helper.start({
uploadStream: dataStream,
resumeUrl: savedUploadUrl,
});
const response = await helper.finished();
const uploadSource = client.getResumableSource(filePath);
const helper = await client.createResumableUpload();
await helper.start({
uploadSource,
resumeUrl: savedUploadUrl,
});
const response = await helper.finished();
References
  1. Do not manually edit auto-generated files to fix typos or make other changes, as these edits will be overwritten during the next regeneration. Instead, apply the fixes upstream in the generator or templates.

The checked-in generated client is linted and type-checked as its own
package, but CI installs the published google-gax (which does not have the
resumable upload APIs yet) and never compiles protos/protos, so the client
reported 12 type errors plus a promise/always-return error on every run.

Treat it as a fixture instead:

- move client/ to fixtures/, a path segment the monorepo linter ignores
- rename its tsconfig.json to tsconfig.client.json, so the package
  detection walks up to google-gax's tsconfig and skips these files
- update run.sh, sample.js, the client package.json and the harness README

Verified by running the harness's own compile steps (compileProtos plus
tsc -p tsconfig.client.json) against the local google-gax checkout.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant