Skip to content

Fix media metadata-only uploads and various documentation fixes. - #230

Merged
ryanseys merged 9 commits into
googleapis:masterfrom
ryanseys:fix-media-metadata
Jul 28, 2014
Merged

ryanseys merged 9 commits into
googleapis:masterfrom
ryanseys:fix-media-metadata

Conversation

@ryanseys

Copy link
Copy Markdown
Contributor

Requires a change to media parameter that breaks old implementation but allows for more flexible media uploads.

media: data

now becomes:

media: {
  mimeType: 'mime here',
  body: data
}

@ryanseys

Copy link
Copy Markdown
Contributor Author

Still have to update README and Migrating.

@ryanseys

Copy link
Copy Markdown
Contributor Author

Possible fix for #229

@rakyll

rakyll commented Jul 26, 2014

Copy link
Copy Markdown
Contributor

JSDoc requires changes as well. It has to go in a major release (1.1.0).

@ryanseys

Copy link
Copy Markdown
Contributor Author

Fixed JSDoc.

@ryanseys

Copy link
Copy Markdown
Contributor Author

Major? http://semver.org/ Major would be 2.0.0. And we are creating changes that break existing functionality. I suppose the change is small enough that it can be neglected as a major.

@robertrossmann

Copy link
Copy Markdown
Contributor

We can do it without breaking backwards compatibility - simply check if the input to the media param has mimeType property and depending on that use either the old or the new logic.

Optionally, we can console.error when the old behaviour is used so that the programmer is informed about this API being changed.

@ryanseys

Copy link
Copy Markdown
Contributor Author

Not reliably enough because a stream object can be provided and for all we know, it could have the mimeType property defined. I would rather not assume and instead just release it under 1.1. Nobody should use the old functionality because it is inherently broken for many APIs anyway. Plus migrating to new changes is rather easy.

@ryanseys

Copy link
Copy Markdown
Contributor Author

Effectively, the issue with 1.0.x is it didn't take into consideration the case of metadata-only media requests. In this fix, we check whether the user has provided a media.body, and if they have, we make the multipart request as normal. If they don't provide a media.body we make a simple json request where the body is just the resource. The Content-Type of the request can only be specified when a media.body is specified, in which case you can set the mime-type using media.mimeType. Fallback mime-type provided by resource.mimeType option, "text/plain" for strings or "application/octet-stream" as last chance.

Comment thread lib/apirequest.js

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@rakyll

rakyll commented Jul 28, 2014

Copy link
Copy Markdown
Contributor

LGTM.

We need a minor release, anyone who uses ~1.0.0 will be able to get the update, whereas if you're strictly depending on 1.0.x, you won't.

ryanseys added a commit that referenced this pull request Jul 28, 2014
Fix media metadata-only uploads and various documentation fixes.
@ryanseys
ryanseys merged commit 3c8d510 into googleapis:master Jul 28, 2014
@ryanseys
ryanseys deleted the fix-media-metadata branch July 28, 2014 20:08
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.

4 participants