13 years, 2 months ago
(2013-08-09 20:03:13 UTC)
#3
https://codereview.appspot.com/12590044/diff/1/google-api-client/src/main/jav...
File
google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpUploader.java
(right):
https://codereview.appspot.com/12590044/diff/1/google-api-client/src/main/jav...
google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpUploader.java:539:
if (!suppressGZip && !disableGZipContent && !(request.getContent() instanceof
EmptyContent)) {
On 2013/08/09 19:05:44, peleyal wrote:
> Consider add a unit test for this logic. I mean that setEncoding is call for
the
> right conditions
Done. I also changed the code to look closer to what it was before.
https://codereview.appspot.com/12590044/diff/1/google-api-client/src/test/jav...
File
google-api-client/src/test/java/com/google/api/client/googleapis/media/MediaHttpDownloaderTest.java
(left):
https://codereview.appspot.com/12590044/diff/1/google-api-client/src/test/jav...
google-api-client/src/test/java/com/google/api/client/googleapis/media/MediaHttpDownloaderTest.java:376:
public void testDirectDownloadServerErrorWithBackOffEnabled() throws Exception {
On 2013/08/09 19:05:44, peleyal wrote:
> I think we should add back this method with
BackOffUnsuccessfulResponseHandler,
> right?
> Like we did in MediaHttpUploader
Done.
13 years, 2 months ago
(2013-08-12 15:46:06 UTC)
#5
https://codereview.appspot.com/12590044/diff/12001/google-api-client/src/main...
File
google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpUploader.java
(right):
https://codereview.appspot.com/12590044/diff/12001/google-api-client/src/main...
google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpUploader.java:417:
response = executeCurrentRequestWithoutGZip(currentRequest);
On 2013/08/12 13:24:55, peleyal wrote:
> I actually prefer you patch1 solution - executeCurrentRequest(currentRequest,
> isMediaLengthKnown());
let's revisit this when we fix
https://code.google.com/p/google-api-java-client/issues/detail?id=691
we might not need execute without gzip at all at that point
https://codereview.appspot.com/12590044/diff/12001/google-api-client/src/test...
File
google-api-client/src/test/java/com/google/api/client/googleapis/media/MediaHttpUploaderTest.java
(right):
https://codereview.appspot.com/12590044/diff/12001/google-api-client/src/test...
google-api-client/src/test/java/com/google/api/client/googleapis/media/MediaHttpUploaderTest.java:388:
static class ZeroBackOffRequestInitializer implements HttpRequestInitializer {
On 2013/08/12 13:24:55, peleyal wrote:
> I hate that MediaHttpDownloader use this one.
> Maybe its a good idea to have a separate class for this one (especially that
> it's not media upload \ download specific?
> Makes sense?
TODO added, though think carefully about whether you actually want to do this
https://codereview.appspot.com/12590044/diff/12001/google-api-client/src/test...
google-api-client/src/test/java/com/google/api/client/googleapis/media/MediaHttpUploaderTest.java:931:
}
On 2013/08/12 13:24:55, peleyal wrote:
> Consider adding back testDirectUploadServerErrorWithBackOffEnabled() but
change
> it to:
> testDirectUploadServerError_WithBackOffUnsuccessfulResponseHandler?
Done.
Issue 12590044: api issue 819: Remove Beta API deprecated in 1.16 [backwards incompatible]
(Closed)
Created 13 years, 2 months ago by yanivi
Modified 13 years, 2 months ago
Reviewers: peleyal
Base URL: https://code.google.com/p/google-api-java-client/
Comments: 14
