You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Gaul <no...@github.com> on 2013/12/02 06:57:39 UTC
[jclouds] Remove BlobBuilder.calculateMD5 (#218)
Callers should instead call BlobBuilder.contentMD5, usually with the
results from Guava Hashing.md5(). This narrows the API and removes a
strange IOException from callers.
You can merge this Pull Request by running:
git pull https://github.com/maginatics/jclouds remove-calculate-md5
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/218
-- Commit Summary --
* Remove BlobBuilder.calculateMD5
-- File Changes --
M apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java (3)
M blobstore/src/main/java/org/jclouds/blobstore/domain/BlobBuilder.java (7)
M blobstore/src/main/java/org/jclouds/blobstore/domain/internal/BlobBuilderImpl.java (5)
M blobstore/src/test/java/org/jclouds/blobstore/TransientBlobRequestSignerTest.java (15)
M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseBlobIntegrationTest.java (6)
M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerIntegrationTest.java (4)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/218.patch
https://github.com/jclouds/jclouds/pull/218.diff
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #889](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/889/) 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/218#issuecomment-29597558
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #656](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/656/) 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/218#issuecomment-29597024
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by Andrew Gaul <no...@github.com>.
Superseded by #381.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/218#issuecomment-44358648
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by Andrew Gaul <no...@github.com>.
Closed #218.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/218#event-125380706
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #888](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/888/) 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/218#issuecomment-29596610
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #657](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/657/) 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/218#issuecomment-29597760
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by Andrew Gaul <no...@github.com>.
We should remove ```calculateMD5``` because it violates the principle of least astonishment. For repeatable payloads like ```InputSupplier``` it incurs IO *twice* and for non-repeatable payloads like ```InputStream``` it *buffers* the entire bolus in-memory, often times causing ```OutOfMemoryError```. We should let the user see the cost, e.g., calling Files.hash, rather than taking magical actions on the user's behalf.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/218#issuecomment-29779783
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #887](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/887/) 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/218#issuecomment-29596258
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by Andrew Phillips <no...@github.com>.
Could you explain the background for this? Where previously jclouds could do work for you (and work that it should seemingly be able to do, i.e. calculate the MD5 for content you've already given it), we now expect you to do the work yourself, and get the implementation right?
Is there any reason the user has to be able to set the MD5 value explicitly? I would think naively that I'd rather kick out the ability to _explicitly set_ the content MD5, and **only** allow you to ask jclouds to calculate it?
Also, if we ultimately do want this to go head, then also remove `Payloads.calculateMD5(payload)`, or do we still need that?
Failures look real, by the way. Something in the clojure binding:
```
ERROR in (large-container-list-test) (Reflector.java:289)
Uncaught exception, not in assertion.
expected: nil
actual: java.lang.IllegalArgumentException: No matching field found: calculateMD5 for class org.jclouds.blobstore.domain.internal.BlobBuilderImpl$PayloadBlobBuilderImpl
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/218#issuecomment-29695167
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #904](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/904/) 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/218#issuecomment-30179092
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #655](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/655/) 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/218#issuecomment-29596622
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #673](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/673/) 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/218#issuecomment-30182741
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by Andrew Phillips <no...@github.com>.
@andrewgaul: irrespective of whether it makes sense to try to fix the implementation rather than remove it - I guess this would count as a breaking change? Or do we feel this is used internally only, so we wouldn't be impacting users here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/218#issuecomment-30170062
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #424](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/424/) 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/218#issuecomment-29596475
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #426](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/426/) 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/218#issuecomment-29597556
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #425](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/425/) 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/218#issuecomment-29596551
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by Andrew Phillips <no...@github.com>.
> For repeatable payloads like InputSupplier it incurs IO twice and for non-repeatable payloads like InputStream it
> buffers the entire bolus in-memory, often times causing OutOfMemoryError
Rather than removing it, would it be feasible to simply fix/amend those implementations to be less astonishing? Otherwise, yes, I can see better what you mean here.
Thanks for explaining!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/218#issuecomment-29785328
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by Andrew Gaul <no...@github.com>.
@demobox Breaking change, plan to deprecate in #223 and push this commit in 1.8.x.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/218#issuecomment-30177117
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by Andrew Phillips <no...@github.com>.
Still the same [clojure failure](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/441/console) by the way, it seems:
```
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal com.theoryinpractise:clojure-maven-plugin:1.3.10:test (test-clojure) on project jclouds-blobstore: Clojure failed
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/218#issuecomment-30187058
Re: [jclouds] Remove BlobBuilder.calculateMD5 (#218)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #441](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/441/) 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/218#issuecomment-30179091