You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Timur Alperovich <no...@github.com> on 2014/06/25 22:21:02 UTC
[jclouds-karaf] Do not cache System.out in commands. (#47)
If the System.out object is cached, callers that have invoked
System.setOut will observe that the new stream is not used and the
original file descriptor continues to be used by jclouds-karaf
commands.
You can merge this Pull Request by running:
git pull https://github.com/maginatics/jclouds-karaf do_not_cache_system.out
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-karaf/pull/47
-- Commit Summary --
* Do not cache System.out in commands.
-- File Changes --
M commands/src/main/java/org/jclouds/karaf/commands/blobstore/BlobListCommand.java (4)
M commands/src/main/java/org/jclouds/karaf/commands/blobstore/BlobMetadataCommand.java (8)
M commands/src/main/java/org/jclouds/karaf/commands/blobstore/ContainerMetadataCommand.java (8)
-- Patch Links --
https://github.com/jclouds/jclouds-karaf/pull/47.patch
https://github.com/jclouds/jclouds-karaf/pull/47.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Phillips <no...@github.com>.
> Pushed to master and 1.7.x.
Thanks, @andrewgaul!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47195558
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Phillips <no...@github.com>.
Closed #47.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#event-135264723
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Phillips <no...@github.com>.
> There is no hurry here -- definitely should have jenkins run against this.
Waiting on https://github.com/jclouds/jclouds-karaf/pull/48 at the moment...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47165775
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Phillips <no...@github.com>.
The change itself looks fine with me - thanks, @timuralp! But let's fix karaf first and let the PR builders run against this once.
Functionally, this makes sense, but do we know what the performance impact could be? @iocanel Any light you could shed on that?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47163868
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-karaf-pull-requests #78](https://jclouds.ci.cloudbees.com/job/jclouds-karaf-pull-requests/78/) 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-karaf/pull/47#issuecomment-47171492
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Gaul <no...@github.com>.
I wrote these commands and cached `System.out` for brevity. Pushed to master and 1.7.x.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47181201
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-karaf-pull-requests #79](https://jclouds.ci.cloudbees.com/job/jclouds-karaf-pull-requests/79/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47176173
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Phillips <no...@github.com>.
Karaf should be fixed. Time to bounce this and try again!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47171349
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Phillips <no...@github.com>.
Weird, indeed. But the current set of CI jobs for Karaf look happy enough:
https://buildhive.cloudbees.com/job/jclouds/job/jclouds-karaf/1070/console
https://jclouds.ci.cloudbees.com/job/jclouds-karaf/642/console
So I think we'll wait for those to finish and try again...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47173992
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Timur Alperovich <no...@github.com>.
@demobox one thing I wanted to point out is that every other command does not cache the System.out object, so I guess this is also consistent :)
And sounds good. There is no hurry here -- definitely should have jenkins run against this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47164782
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Timur Alperovich <no...@github.com>.
I'm not sure why the cloud bees test failed in the ComputeServiceProxy. Could someone shed some light onto it? I see the following errors:
<code>
ComputeServiceEventProxy.java:[275,4] method does not override or implement a method from a supertype
</code>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47153893
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Phillips <no...@github.com>.
Reopened #47.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#event-135277465
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Phillips <no...@github.com>.
Reopened #47.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#event-135264728
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-karaf #1064](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-karaf/1064/) 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-karaf/pull/47#issuecomment-47153509
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Phillips <no...@github.com>.
> May the failure be caused by jclouds/jclouds#419 ?
It indeed looks like [this commit](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;a=commit;h=8598ee858adf05246124a7a7db0de26e191c3fb1) broke jclouds-karaf, but our CI build aborted, so we didn't notice. Will have a look at that now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47161731
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-karaf #1069](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-karaf/1069/) 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-karaf/pull/47#issuecomment-47171432
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-karaf-pull-requests #76](https://jclouds.ci.cloudbees.com/job/jclouds-karaf-pull-requests/76/) 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-karaf/pull/47#issuecomment-47153587
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Andrew Phillips <no...@github.com>.
OK, here goes...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47174988
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-karaf #1071](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-karaf/1071/) 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-karaf/pull/47#issuecomment-47178162
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Timur Alperovich <no...@github.com>.
@iocanel @andrewgaul suggested you're the best person to look at this pool request. Let me know if there are changes I should make here or any other comments. Also, would it be possible to have the change backported to 1.7.x as well if it's merged?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47161725
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Ignasi Barrera <no...@github.com>.
May the failure be caused by https://github.com/jclouds/jclouds/pull/419 ?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47157015
Re: [jclouds-karaf] Do not cache System.out in commands. (#47)
Posted by Timur Alperovich <no...@github.com>.
Looks like more errors in the computeService?
>Waiting for Jenkins to finish collecting data[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.0.2:compile (default-compile) on project core: Compilation failure: Compilation failure:
[ERROR] /scratch/jenkins/workspace/jclouds-karaf-pull-requests/core/src/main/java/org/jclouds/karaf/core/ComputeServiceEventProxy.java:[57,7] org.jclouds.karaf.core.ComputeServiceEventProxy is not abstract and does not override abstract method rebootNodesMatching(com.google.common.base.Predicate<org.jclouds.compute.domain.NodeMetadata>) in org.jclouds.compute.ComputeService
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/47#issuecomment-47173093