Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow an empty requestBody to be provided for APIs that support … #1988

Merged
merged 3 commits into from
Mar 11, 2020

Conversation

bcoe
Copy link
Contributor

@bcoe bcoe commented Mar 11, 2020

This PR updates the generator such that an empty requestBody can be provided for methods that support multipart uploads (determined by checking m.mediaUpload.protocols.simple.multipart).

Most of the Google APIs I looked at require that the first section of the multipart POST represents metadata about the upload, e.g.,

Metadata part. Must come first, and must have a Content-Type header set to application/json; charset=UTF-8. Add the file's metadata to this part in JSON format.

In testing with the youtube's thumbnails.set, the upstream API requires that empty metadata be set to perform a multi-part upload (it's my hunch that this behavior will be fairly consistent across APIs). You can now perform a multipart thumbnail upload like so:

    await youtube.thumbnails.set(
      {
        videoId: 'abc123',
        requestBody: {},
        media: {
          mimeType: 'image/jpeg',
          body: fs.createReadStream(fileName),
        },
      },
      {
        onUploadProgress: (evt: {bytesRead: number}) => {
          progressEvents.push(evt.bytesRead);
        },
      }
    );

☝️ this will fire onUploadProgress events.

fixes: #1820

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 11, 2020
@bcoe bcoe requested review from JustinBeckwith and feywind March 11, 2020 16:21
@bcoe bcoe changed the title fix: allow an empty requestBody to be provided for APIs that support … fix!: allow an empty requestBody to be provided for APIs that support … Mar 11, 2020
@@ -411,13 +411,9 @@ export namespace accesscontextmanager_v1beta {
*/
unrestrictedServices?: string[] | null;
/**
* Beta. Configuration for within Perimeter allowed APIs.
* Beta. Configuration for APIs allowed within Perimeter.
Copy link
Contributor

Choose a reason for hiding this comment

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

Eugh. Could I trouble you to first submit and land a re-generate PR from master? Would love to see only the relevant changes in this PR. Sorry.

/**
* Alpha. Specifies how APIs are allowed to communicate within the Service Perimeter. This message is DEPRECATED and had been renamed to VpcAccessibleServices
*/
export interface Schema$VpcServiceRestriction {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

most of the updates below are the result of running the generator, which tends to change a few existing APIs, regardless of changes to the templates.

const progressEvents = new Array<number>();
await youtube.thumbnails.set(
{
videoId: 'abc123',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test demonstrates how to perform a multipart upload with an API that doesn't expect a requestBody.

@@ -154,6 +154,11 @@ export interface Schema${{ schema.id }} {
* Request body metadata
*/
requestBody?: Schema${{ m.request.$ref }};
{% elif m.supportsMediaUpload and m.mediaUpload.protocols.simple.multipart %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the meat of the change.

@codecov
Copy link

codecov bot commented Mar 11, 2020

Codecov Report

Merging #1988 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1988   +/-   ##
=======================================
  Coverage   38.76%   38.76%           
=======================================
  Files           6        6           
  Lines        1068     1068           
  Branches        8        6    -2     
=======================================
  Hits          414      414           
  Misses        654      654           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8bcb212...4bbbb8e. Read the comment docs.

@bcoe bcoe changed the title fix!: allow an empty requestBody to be provided for APIs that support … fix: allow an empty requestBody to be provided for APIs that support … Mar 11, 2020
@bcoe bcoe merged commit 074f641 into master Mar 11, 2020
@bcoe bcoe deleted the fix-1820 branch March 11, 2020 21:33
gcf-merge-on-green bot pushed a commit that referenced this pull request Mar 11, 2020
🤖 I have created a release \*beep\* \*boop\* 
---
## [48.0.0](https://github.com/googleapis/google-api-nodejs-client/compare/v47.0.0...v48.0.0) (2020-03-11)


### ⚠ BREAKING CHANGES

* removes toolresults_v1.
* regenerate all APIs (#1978)

### Features

* regenerate all APIs ([#1978](https://github.com/googleapis/google-api-nodejs-client/issues/1978)) ([f0d4913](https://github.com/googleapis/google-api-nodejs-client/commit/f0d49136eaa12838a74a56aa45e08fa870278ae5))
* run the generator (adds: displayvideo, gamesConfiguration, managedidentities, networkmanagement) ([#1989](https://github.com/googleapis/google-api-nodejs-client/issues/1989)) ([8bcb212](https://github.com/googleapis/google-api-nodejs-client/commit/8bcb212fbab43a1e3214da4712b4c3363d1b1285))


### Bug Fixes

* **deps:** update dependency uuid to v7 ([#1970](https://github.com/googleapis/google-api-nodejs-client/issues/1970)) ([fdf096e](https://github.com/googleapis/google-api-nodejs-client/commit/fdf096ee80c87a98b7d20666a2e38996228fbaf1))
* allow an empty requestBody to be provided for APIs that support multipart post ([#1988](https://github.com/googleapis/google-api-nodejs-client/issues/1988)) ([074f641](https://github.com/googleapis/google-api-nodejs-client/commit/074f6417754930cbcbf5589bbcb88549b9f430a9))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Youtube set thumbnail, onUploadProgress doesn't work
3 participants