Closed #1120.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1120#event-1215196971
Pushed to master as e331a000d17d36043165cf79c505a7c0a7370f05. I still do not
understand the usefulness but there is no technical argument against this
change. If you want to improve on this commit you could investigate which
providers actually need/want 100-continue and provide configuration
@andrewgaul Currently, I don't have any performance numbers with me. As I have
mentioned before, we were comparing the no. of requests being made for a
multi-part upload using S3Proxy vs using Azure directly, where I've encountered
this redundant request and raised the PR to fix the same.
--
Thanks for investigating the failure which I addressed in
9e73bbec16cbf91c5f228d4c3ae99c6c526081f8. You will need to rebase your commit
since I included your integration tests to validate my fix. I guess this is
fine to merge although I still do not understand why you are so motivated to
@andrewgaul This PR doesn't introduce any regression. `putBlob` with
zero-length inputstream is failing before as well on google-cloud. The thing is
we don't have a test case for that scenario before, so you didn't see any test
failure.
--
You are receiving this because you are subscribed to
To clear up any confusion, the proposed change introduces a regression in
google-cloud-storage and must be addressed before merging. Previously
zero-byte writes succeeded and with this pull request they fail.
--
You are receiving this because you are subscribed to this thread.
Reply to this
@ChaithanyaGK pushed 1 commit.
dad7d0a Code review changes
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1120/files/22692e66b2ddc8adec4f55a74cbe9f1f987da28c..dad7d0a8f95829b8b96e54e1a3337f4cf7d585e3
@andrewgaul Could you please let me know is there anything else that needs to
be addressed here.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1120#issuecomment-317779896
> You should add a test for putting a zero-length blob to
> BaseBlobStoreIntegrationTest and test this against a few providers. We
> probably want a second test for streaming zero-length InputStream payloads as
> well.
@andrewgaul I've added integration tests for zero-length `String`,
@ChaithanyaGK pushed 1 commit.
22692e6 Code cleanup and added integration tests
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
> > could you explain the motivation for this change? I don't understand what
> > this addresses except optimizing the rare case of a zero-length PUT.
>
> @andrewgaul Every time I initiate a multipart upload to azure using S3Proxy
> it creates an empty blob, where I've encountered this issue.
ChaithanyaGK commented on this pull request.
> @@ -779,6 +782,22 @@ private static void addHeader(Multimap
> headers, Headers header,
return parts.build();
}
+ private static GeneratedHttpRequest
stripExpectHeaderIfContentZero(GeneratedHttpRequest request) {
+
> could you explain the motivation for this change? I don't understand what
> this addresses except optimizing the rare case of a zero-length PUT.
@andrewgaul Every time I initiate a multipart upload to azure using S3Proxy it
creates an [empty
ChaithanyaGK commented on this pull request.
> @@ -73,7 +74,7 @@ public void testZeroLengthPutHasContentLengthHeader()
> throws IOException, Interr
RecordedRequest request = server.takeRequest();
assertEquals(request.getRequestLine(), "PUT /bucket/object HTTP/1.1");
14 matches
Mail list logo