Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(395)

Issue 13480044: Issue 361: MediaDownloader can't download drive export list (which includes query parameters) (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 6 months ago by peleyal
Modified:
10 years, 5 months ago
Reviewers:
ngmiceli
CC:
google-api-dotnet-client_googlegroup.com
Base URL:
https://google-api-dotnet-client.googlecode.com/hg/
Visibility:
Public.

Description

Issue 361: MediaDownloader can't download drive export list (which includes query parameters)

Patch Set 1 #

Patch Set 2 : Support also issue 387: add NumRedirects to HttpConfigurableMessageHandler #

Patch Set 3 : minor #

Patch Set 4 : minor #

Total comments: 8

Patch Set 5 : Nick comments #

Patch Set 6 : minor #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+91 lines, -13 lines) Patch
M Src/GoogleApis.Tests/Apis/Download/MediaDownloaderTest.cs View 2 chunks +13 lines, -3 lines 0 comments Download
M Src/GoogleApis.Tests/Apis/Http/ConfigurableMessageHandlerTest.cs View 1 2 3 4 2 chunks +4 lines, -3 lines 0 comments Download
M Src/GoogleApis.Tests/Apis/Requests/ClientServiceRequestTest.cs View 1 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
M Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs View 1 2 3 4 5 5 chunks +36 lines, -1 line 1 comment Download
M Src/GoogleApis/Apis/[Media]/Download/MediaDownloader.cs View 1 2 3 4 3 chunks +35 lines, -3 lines 0 comments Download

Messages

Total messages: 4
peleyal
10 years, 6 months ago (2013-09-14 01:22:49 UTC) #1
ngmiceli
https://codereview.appspot.com/13480044/diff/12001/Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs File Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs (right): https://codereview.appspot.com/13480044/diff/12001/Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs#newcode97 Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs:97: /// only the allowed tries of >=400 responses, or ...
10 years, 5 months ago (2013-10-04 20:12:22 UTC) #2
peleyal
https://codereview.appspot.com/13480044/diff/12001/Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs File Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs (right): https://codereview.appspot.com/13480044/diff/12001/Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs#newcode97 Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs:97: /// only the allowed tries of >=400 responses, or ...
10 years, 5 months ago (2013-10-10 20:06:37 UTC) #3
ngmiceli
10 years, 5 months ago (2013-10-14 19:27:10 UTC) #4
LGTM

Just patch up documentation

https://codereview.appspot.com/13480044/diff/22001/Src/GoogleApis/Apis/Http/C...
File Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs (right):

https://codereview.appspot.com/13480044/diff/22001/Src/GoogleApis/Apis/Http/C...
Src/GoogleApis/Apis/Http/ConfigurableMessageHandler.cs:98: /// <see
cref="NumTries"/> to 1 and <see cref="NumRedirect"/> to 5, a retry will happen
only for 5 redirects.
Try to be as explicit as possible. Here's what we came up with together:

"to 5, the library will send up to five redirect requests, but will not send any
retry requests due to an error HTTP status code."
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b