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/27 17:15:01 UTC
[jclouds] Revert requiring guava 17 (#588)
see https://issues.apache.org/jira/browse/JCLOUDS-534 for discussion
You can merge this Pull Request by running:
git pull https://github.com/adriancole/jclouds adrian.less-guava-incompatibility
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/588
-- Commit Summary --
* JCLOUDS-534 Avoid runtime incompatibility introduced by Guava's closeQuietly.
* JCLOUDS-534 Folded overly explicit use of HostAndPort, which is also incompatible with guava 16.
* JCLOUDS-534 Set floor version of guava to 16.0.1
-- File Changes --
M apis/byon/src/main/java/org/jclouds/byon/functions/NodesFromYamlStream.java (10)
M apis/chef/src/main/java/org/jclouds/chef/handlers/ChefErrorHandler.java (14)
M apis/chef/src/test/java/org/jclouds/chef/internal/BaseChefApiLiveTest.java (11)
M apis/elasticstack/src/main/java/org/jclouds/elasticstack/handlers/ElasticStackErrorHandler.java (13)
M apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java (4)
M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerLiveTest.java (2)
M core/src/main/java/org/jclouds/JcloudsVersion.java (6)
M core/src/main/java/org/jclouds/http/internal/JavaUrlHttpCommandExecutorService.java (13)
M core/src/main/java/org/jclouds/io/ByteStreams2.java (7)
M core/src/main/java/org/jclouds/util/Closeables2.java (35)
M core/src/test/java/org/jclouds/http/BackoffLimitedRetryJavaTest.java (12)
M core/src/test/java/org/jclouds/http/BaseHttpCommandExecutorServiceIntegrationTest.java (62)
M core/src/test/java/org/jclouds/http/JavaUrlHttpCommandExecutorServiceIntegrationTest.java (4)
M drivers/okhttp/src/test/java/org/jclouds/http/okhttp/OkHttpCommandExecutorServiceTest.java (10)
M project/pom.xml (2)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/588.patch
https://github.com/jclouds/jclouds/pull/588.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1362](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1362/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588#issuecomment-60992337
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Adrian Cole <no...@github.com>.
> import org.jclouds.javax.annotation.Nullable;
>
> -@Deprecated
> -public class Closeables2 {
> - private Closeables2() {
> - }
> +import com.google.common.annotations.Beta;
> +
> +/**
> + * Alternative to {@link org.jclouds.util.Closeables2}, which allows jclouds
Whoops find replace error!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588/files#r19517645
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Adrian Cole <no...@github.com>.
> @@ -76,7 +76,7 @@ public boolean apply(@Nullable StorageMetadata input) {
>
> SocketOpen socketOpen = context.utils().injector().getInstance(SocketOpen.class);
> Predicate<HostAndPort> socketTester = retry(socketOpen, 1200, 10, SECONDS);
> - assertTrue(socketTester.apply(HostAndPort.fromHost(metadata.getPublicUri().getHost()).withDefaultPort(80)), metadata.getPublicUri().toString());
> + assertTrue(socketTester.apply(HostAndPort.fromParts(metadata.getPublicUri().getHost(), 80)), metadata.getPublicUri().toString());
checked the code and these two are equiv. ex. fromHost throws on port in the host section, so does fromParts.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588/files#r19417062
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Ignasi Barrera <no...@github.com>.
@adriancole in 1.8.x Chef is still in the jclouds-chef repo. I didn't promote it because the release was in progress and I forgot once it was released.
I'll promote it tonight but I might have lost track of the changes that need to be applied in the jclouds-chef repo's 1.8.x branch. Mind having a look before I promote it?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588#issuecomment-61058965
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Adrian Cole <no...@github.com>.
merged into master, 1.8.x
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588#issuecomment-61027455
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1883](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1883/) 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/588#issuecomment-60997905
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-7 #267](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-7/267/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588#issuecomment-60625385
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Andrew Phillips <no...@github.com>.
One minor comment, otherwise looks good to me. That's all we gained with Guava 17?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588#issuecomment-61024328
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Andrew Phillips <no...@github.com>.
> otherwise looks good to me
I forgot the "+1", sorry ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588#issuecomment-61036519
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-7 #273](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-7/273/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588#issuecomment-60991292
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1872](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1872/) 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/588#issuecomment-60642956
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Andrew Phillips <no...@github.com>.
> @@ -67,13 +63,7 @@ public void handleError(HttpCommand command, HttpResponse response) {
> break;
> }
> } finally {
> - if (response.getPayload() != null) {
> - try {
> - Closeables.close(response.getPayload().getInput(), true);
> - } catch (IOException e) {
> - throw propagate(e);
> - }
> - }
> + closeQuietly(response.getPayload());
Difference in behaviour here (swallowed vs. propagated exception) - is that OK?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588/files#r19580477
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-7 #266](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-7/266/) 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/588#issuecomment-60620989
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Adrian Cole <ad...@gmail.com>.
Great!
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Ignasi Barrera <no...@github.com>.
I've just seen there is a PR in Chef :) Thx! I'll promote it tonight, to have all "live" branches in the same repo.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588#issuecomment-61059272
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Adrian Cole <no...@github.com>.
> try {
> - Closeables.close(closeable, true);
> - } catch (IOException e) {
> - throw Throwables.propagate(e);
ps. this wasn't possible!!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588/files#r19417014
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1355](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1355/) UNSTABLE
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/588#issuecomment-60624591
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Andrew Phillips <no...@github.com>.
> import org.jclouds.javax.annotation.Nullable;
>
> -@Deprecated
> -public class Closeables2 {
> - private Closeables2() {
> - }
> +import com.google.common.annotations.Beta;
> +
> +/**
> + * Alternative to {@link org.jclouds.util.Closeables2}, which allows jclouds
Alternative to `org.jclouds.util.Closeables2`? Isn't that this class?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588/files#r19517154
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Adrian Cole <no...@github.com>.
> @@ -67,13 +63,7 @@ public void handleError(HttpCommand command, HttpResponse response) {
> break;
> }
> } finally {
> - if (response.getPayload() != null) {
> - try {
> - Closeables.close(response.getPayload().getInput(), true);
> - } catch (IOException e) {
> - throw propagate(e);
> - }
> - }
> + closeQuietly(response.getPayload());
nope. the true flag meant swallow IOE, so this form was accidentally redundant.
http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/io/Closeables.html#close(java.io.Closeable,%20boolean)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588/files#r19581119
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by Andrew Phillips <no...@github.com>.
> @@ -67,13 +63,7 @@ public void handleError(HttpCommand command, HttpResponse response) {
> break;
> }
> } finally {
> - if (response.getPayload() != null) {
> - try {
> - Closeables.close(response.getPayload().getInput(), true);
> - } catch (IOException e) {
> - throw propagate(e);
> - }
> - }
> + closeQuietly(response.getPayload());
> the true flag meant swallow IOE, so this form was accidentally redundant.
Ah, _that's_ what you meant by the "not possible" comment elsewhere. Thanks for explaining!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588/files#r19584839
Re: [jclouds] Revert requiring guava 17 (#588)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1356](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1356/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/588#issuecomment-60626252