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.

Sign up for GitHub

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

Jump to bottom

Fixes for downloads #622

Merged
merged 3 commits into from Nov 5, 2015
Merged

Fixes for downloads #622

merged 3 commits into from Nov 5, 2015

Conversation

sqrrrl
Copy link
Contributor

@sqrrrl sqrrrl commented Nov 3, 2015

Three fixes related to downloads:

  • Ensure query parameters are decoded properly before being re-encoded by RequestBuilder
  • Handle query params without values correctly (fix index out of range error)
  • Check the response code before streaming the download and report any non-success codes as failed downloaded.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 3, 2015
peleyal
peleyal reviewed Nov 4, 2015
View reviewed changes
Src/GoogleApis/Apis/[Media]/Download/MediaDownloader.cs
if (!response.IsSuccessStatusCode)
{
var error = await service.DeserializeError(response).ConfigureAwait(false);
throw new GoogleApiException(service.Name, error.ToString())

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@peleyal
Copy link
Collaborator

peleyal commented Nov 4, 2015

Thanks for doing this change. I think that I've just nits 👍

@sqrrrl
Copy link
Contributor Author

sqrrrl commented Nov 4, 2015

Cleaned things up, should be better now.

@peleyal
Copy link
Collaborator

peleyal commented Nov 4, 2015

Awesome work!
Before merging this pull request, can you verify that your initial sample (the one that didn't work before) is working now with the new code? Ping me if you need help to set it up.
I know that your test covers it, but it just to double check...

@peleyal peleyal added this to the 1.10 milestone Nov 4, 2015
@sqrrrl
Copy link
Contributor Author

sqrrrl commented Nov 5, 2015

Yep, was able to pull the client lib projects in to my solution and build against that. Seems OK now.

peleyal added a commit that referenced this pull request Nov 5, 2015
@peleyal
Merge pull request #622 from sqrrrl/master
f7e8f44 
Media Download fix - Report errors and ensure query parameters are encoded properly
@peleyal peleyal merged commit f7e8f44 into googleapis:master Nov 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
1.10
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sqrrrl @peleyal @googlebot