Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-08-21 Thread Andrew Gaul
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

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-08-21 Thread Andrew Gaul
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

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-08-07 Thread Chaithanya Ganta
@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. --

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-08-06 Thread Andrew Gaul
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

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-08-06 Thread Chaithanya Ganta
@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

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-08-06 Thread Andrew Gaul
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

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-08-04 Thread Chaithanya Ganta
@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

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-07-25 Thread Chaithanya Ganta
@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

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-07-20 Thread Chaithanya Ganta
> 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`,

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-07-20 Thread Chaithanya Ganta
@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:

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-07-20 Thread Andrew Gaul
> > 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.

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-07-20 Thread Chaithanya Ganta
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) { +

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-07-20 Thread Chaithanya Ganta
> 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

Re: [jclouds/jclouds] JCLOUDS-1322: Remove Expect header for requests with empty body (#1120)

2017-07-20 Thread Chaithanya Ganta
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");