You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Adrian Cole <no...@github.com> on 2014/10/29 00:05:16 UTC

[jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

This reverts the commits which immediately switched String, byte[]. and File-based payloads to require wrapping with Guava ByteSource.

Adding ByteSource support is nice for guava users, but it is unnecessary to complicate others in order to support that.

Remember, we have diverse users and some don&#39;t think Guava is the only way to do I/O.
You can merge this Pull Request by running:

  git pull https://github.com/adriancole/jclouds adrian.revert-bytes

Or you can view, comment on it, or merge it online at:

  https://github.com/jclouds/jclouds/pull/592

-- Commit Summary --

  * Revert &quot;Replace uses of ByteArrayPayload&quot;
  * Revert &quot;Remove ByteArrayPayload&quot;
  * Do not require Guava ByteSource in order to create a payload.

-- File Changes --

    M blobstore/src/main/clojure/org/jclouds/blobstore2.clj (2)
    M blobstore/src/main/java/org/jclouds/blobstore/domain/BlobBuilder.java (6)
    M blobstore/src/main/java/org/jclouds/blobstore/domain/internal/BlobBuilderImpl.java (5)
    M blobstore/src/main/java/org/jclouds/blobstore/strategy/internal/MarkerFileMkdirStrategy.java (7)
    M core/src/main/java/org/jclouds/http/HttpMessage.java (9)
    M core/src/main/java/org/jclouds/http/HttpUtils.java (3)
    M core/src/main/java/org/jclouds/io/Payloads.java (24)
    M core/src/main/java/org/jclouds/io/internal/BasePayloadSlicer.java (4)
    A core/src/main/java/org/jclouds/io/payloads/ByteArrayPayload.java (45)
    M core/src/main/java/org/jclouds/io/payloads/FilePayload.java (4)
    M core/src/main/java/org/jclouds/io/payloads/StringPayload.java (4)
    M core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java (2)
    M drivers/apachehc/src/main/java/org/jclouds/http/apachehc/ApacheHCUtils.java (6)
    M drivers/gae/src/main/java/org/jclouds/gae/ConvertToJcloudsResponse.java (2)
    M providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3ClientLiveTest.java (4)
    M providers/azureblob/src/test/java/org/jclouds/azureblob/AzureBlobClientLiveTest.java (8)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/592.patch
https://github.com/jclouds/jclouds/pull/592.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by Andrew Phillips <no...@github.com>.
> @@ -358,9 +358,9 @@ public void testBlockOperations() throws Exception {
>        String blockIdB = BaseEncoding.base64().encode((blockBlob + "-" + B).getBytes());
>        String blockIdC = BaseEncoding.base64().encode((blockBlob + "-" + C).getBytes());
>        getApi().createContainer(blockContainer);
> -      getApi().putBlock(blockContainer, blockBlob, blockIdA, Payloads.newPayload(A.getBytes()));
> -      getApi().putBlock(blockContainer, blockBlob, blockIdB, Payloads.newPayload(B.getBytes()));
> -      getApi().putBlock(blockContainer, blockBlob, blockIdC, Payloads.newPayload(C.getBytes()));
> +      getApi().putBlock(blockContainer, blockBlob, blockIdA, new ByteArrayPayload(A.getBytes()));
> +      getApi().putBlock(blockContainer, blockBlob, blockIdB, new ByteArrayPayload(B.getBytes()));
> +      getApi().putBlock(blockContainer, blockBlob, blockIdC, new ByteArrayPayload(C.getBytes()));

Same minor as above about `Payloads.newByteArrayPayload`

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592/files#r19547190

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by Adrian Cole <no...@github.com>.
cherry-picked into master and relevant parts into 1.8.x

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60972219

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1359](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1359/) FAILURE
Looks like there's a problem with this pull request

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60847590

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by Adrian Cole <no...@github.com>.
Closed #592.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#event-185479207

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1881](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1881/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60962890

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by Andrew Phillips <no...@github.com>.
I'm not against putting these methods back but would like to have a bit of a better understanding of why they were removed in the first place. Thanks for getting to grips with this, @adriancole!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60864363

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-7 #271](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-7/271/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60852253

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-7 #270](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-7/270/) FAILURE
Looks like there's a problem with this pull request

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60847468

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1878](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1878/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60850690

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-7 #272](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-7/272/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60955007

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1879](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1879/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60856163

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by Adrian Cole <ad...@gmail.com>.
I wil fix whatever broke in a bit.

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by Adrian Cole <no...@github.com>.
Looks like as a part of addressing https://issues.apache.org/jira/browse/JCLOUDS-410, a decision was made to remove support for other things when addressing `InputSupplier<InputStream>`. I think that decision wasn't made with enough context, and certainly the jira title didn't hint at what was going on there.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60941931

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by Andrew Phillips <no...@github.com>.
@adriancole Thanks for the link to that issue. From that, the global context here seems to be "We plan to transition Payload to ByteSource in the next major release.", and in the light of discussions around being more permissive about Guava versions and Guava usage in general, that needs revisiting.

+1 - looks good to me.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60947580

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1361](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1361/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60956212

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by Adrian Cole <ad...@gmail.com>.
Also note that admitting ByeArrayPayload and StringPayload allow http
drivers like apachehc netty and okhttp avoid rebuffering while switching to
their payload types. Since bytesource is opaque it doesn't allow this.

We need a lot of work on non default http drivers and reverting this helps!

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1360](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1360/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592#issuecomment-60852760

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by Adrian Cole <no...@github.com>.
> @@ -137,6 +139,10 @@ public void addEntityForContent(HttpEntityEnclosingRequest apacheRequest, Payloa
>        } else if (payload instanceof FilePayload) {
>           apacheRequest.setEntity(new FileEntity((File) payload.getRawContent(), payload.getContentMetadata()
>                 .getContentType()));
> +      } else if (payload instanceof ByteArrayPayload) {

@demobox FYI, this is what I was referring to. When we only accept ByteSource, what it is wrapping is opaque. Drivers like this can no longer more efficiently send a string, byte array, etc. This code was pulled out when ByteSource support was adding. This pull request puts it back.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592/files#r19543967

Re: [jclouds] Do not require Guava ByteSource in order to create a payload. (#592)

Posted by Andrew Phillips <no...@github.com>.
> @@ -119,7 +119,7 @@ private Payload createPayload(byte[] content) {
>           Payload payload = null;
>  
>           if (content.length > 0) {
> -            payload = Payloads.newPayload(content);
> +            payload = new ByteArrayPayload(content);

[minor] `Payloads.newByteArrayPayload`?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/592/files#r19547096