Re: [jclouds] Fixed NPE when Server has no Image (JCLOUDS-558) (#421)

2014-06-24 Thread Andrew Phillips
> @@ -174,9 +178,15 @@ protected Hardware > findHardwareForServerOrNull(ServerInZone serverInZone) { > } > > protected OperatingSystem findOperatingSystemForServerOrNull(ServerInZone > serverInZone) { > - Image image = findObjectOfTypeForServerOrNull(images.get(), "image", > serve

Re: [jclouds] Fixed NPE when Server has no Image (JCLOUDS-558) (#421)

2014-06-24 Thread Andrew Phillips
Minor comment, but pending PR builders +1, looks good to me. Thanks, @everett-toews! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/421#issuecomment-47028046

Re: [jclouds] JCLOUDS-610: reboot, suspend, and resumeNodesMatching methods gain functionality similar to that of destroyNodesMatching (#419)

2014-06-24 Thread Andrew Phillips
> jclouds-java-7-pull-requests #1402 UNSTABLE [Spurious test failure](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds$jclouds-compute/1402/testReport/junit/org.jclouds.compute.callables/BlockUntilInitScriptStatusIsZeroThenReturnOutputTest/testloopUntilTrueOrTh

Re: [jclouds] JCLOUDS-610: reboot, suspend, and resumeNodesMatching methods gain functionality similar to that of destroyNodesMatching (#419)

2014-06-24 Thread Andrew Phillips
Closed #419. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/419#event-134752995

Re: [jclouds] JCLOUDS-610: reboot, suspend, and resumeNodesMatching methods gain functionality similar to that of destroyNodesMatching (#419)

2014-06-24 Thread Andrew Phillips
Committed to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;h=8598ee8). Thanks, @cdancy! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/419#issuecomment-47029964

Re: [jclouds] Fixed NPE when Server has no Image (JCLOUDS-558) (#421)

2014-06-24 Thread Andrew Phillips
One minor [Checkstyle violation](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1404/org.apache.jclouds.common$openstack-common/violations/) in openstack-common, but unrelated to this commit. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jcl

Re: [jclouds] JCLOUDS-584 - Update HP Cloud object storage provider (#394)

2014-06-24 Thread Andrew Phillips
> @@ -24,4 +25,13 @@ > public HPCloudObjectStorageContainerLiveTest() { >provider = "hpcloud-objectstorage"; > } > + > + @Override > + @Test > + public void testPublicAccessInNonDefaultLocation() { throw new > SkipException("Locations are ignored"); } [minor] Do we prefer `S

Re: [jclouds] JCLOUDS-584 - Update HP Cloud object storage provider (#394)

2014-06-24 Thread Andrew Phillips
> + fail("static test Strings do not parse to URI: " + ex.getMessage()); > + } > + > + expect(mockSupplier.get()) > + .andReturn(endpoints) > + .anyTimes(); > + > expect(mockFactory.createForApiTypeAndVersion(ServiceType.OBJECT_STORE, null)) > + .andR

Re: [jclouds] JCLOUDS-584 - Update HP Cloud object storage provider (#394)

2014-06-24 Thread Andrew Phillips
+1, looks good to me, too. Thanks, @ccustine! As @everett-toews said: good to see you around! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/394#issuecomment-47039729

Re: [jclouds] JCLOUDS-612: Securely create temporary directories (#420)

2014-06-25 Thread Andrew Phillips
@andrewgaul: Are you around to commit this? Otherwise, I should be able to find some time this evening... --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/420#issuecomment-47127709

Re: [jclouds] JCLOUDS-610: reboot, suspend, and resumeNodesMatching methods gain functionality similar to that of destroyNodesMatching (#419)

2014-06-25 Thread Andrew Phillips
Do we want to/can we backport this to 1.7.x, or is this a backwards-incompatible change? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/419#issuecomment-47161034

Re: [jclouds-karaf] Do not cache System.out in commands. (#47)

2014-06-25 Thread Andrew Phillips
> 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. -

Re: [jclouds] JCLOUDS-612: Securely create temporary directories (#420)

2014-06-25 Thread Andrew Phillips
Merged to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;a=commit;h=a68eb389010fafbdd62ced416ef632cd8cc78844). @andrewgaul @everett-toews Do we want to backport this to 1.7.x? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/420#issue

Re: [jclouds] JCLOUDS-610: reboot, suspend, and resumeNodesMatching methods gain functionality similar to that of destroyNodesMatching (#419)

2014-06-25 Thread Andrew Phillips
> We may want to fix karat first? Indeed. About to push a branch and open a PR. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/419#issuecomment-47163004

Re: [jclouds-karaf] Do not cache System.out in commands. (#47)

2014-06-25 Thread Andrew Phillips
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

[jclouds-karaf] Aligning overrides in ComputeServiceEventProxy with ComputeService (#48)

2014-06-25 Thread Andrew Phillips
Follow-up to https://github.com/jclouds/jclouds/pull/419 You can merge this Pull Request by running: git pull https://github.com/jclouds/jclouds-karaf fix-karaf Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-karaf/pull/48 -- Commit Summary -- *

Re: [jclouds-karaf] Do not cache System.out in commands. (#47)

2014-06-25 Thread Andrew Phillips
> 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-examples] Remove trailing whitespace (#54)

2014-06-25 Thread Andrew Phillips
Scanned through briefly, looks good to me: +1. @everett-toews @zack-shoylev @jdaggett Looks OK to you, too? Thanks, @maxlinc! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/54#issuecomment-47168816

Re: [jclouds-karaf] Aligning overrides in ComputeServiceEventProxy with ComputeService (#48)

2014-06-25 Thread Andrew Phillips
> jclouds-karaf-pull-requests #77 UNSTABLE No [test failures](https://jclouds.ci.cloudbees.com/job/jclouds-karaf-pull-requests/77/testReport/), just an unhappy Checkstyle. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/48#issuecomment-471689

Re: [jclouds-karaf] Aligning overrides in ComputeServiceEventProxy with ComputeService (#48)

2014-06-25 Thread Andrew Phillips
Merged to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds-karaf.git;h=3f1ed2f) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/48#issuecomment-47171309

Re: [jclouds-karaf] Aligning overrides in ComputeServiceEventProxy with ComputeService (#48)

2014-06-25 Thread Andrew Phillips
Closed #48. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/48#event-135264553

Re: [jclouds-karaf] Do not cache System.out in commands. (#47)

2014-06-25 Thread Andrew Phillips
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)

2014-06-25 Thread Andrew Phillips
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)

2014-06-25 Thread Andrew Phillips
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-labs-aws] Bug fix for ContentRange equals (#24)

2014-06-25 Thread Andrew Phillips
Doh! How did that slip through? Thanks for fixing that, @rcoedo! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/24#issuecomment-47171648

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> @@ -16,3 +16,4 @@ TAGS > .metadata/ > atlassian-ide-plugin.xml > .DS_Store > +*.patch Why is this needed? Patch files shouldn't really be part of a standard workspace..? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/61/files#r14218613

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> @@ -0,0 +1,7 @@ > +Apache jclouds labs > +Copyright 2009-2014 The Apache Software Foundation > + > +This product includes software developed at > +The Apache Software Foundation (http://www.apache.org/). > + > +The VMWare vSphere API includes software developed at Cisco Systems, Inc What are we

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> + under the License. > +--> > + > +http://maven.apache.org/POM/4.0.0"; > xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; > + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 > http://maven.apache.org/maven-v4_0_0.xsd";> > + > + 4.0.0 > + > +org.apache.jclouds.labs >

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > + KIND, either express or implied. See the License for the > + specific language governing permissions and limitations > + under the License. > +--> > + > +http://maven.apache.org/POM/4.0.0"; > xmlns:xsi="http://www.w3.org/2001/

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> +org.apache.jclouds.labs > +jclouds-labs > +1.8.0-SNAPSHOT > + > + org.apache.jclouds.api > + vsphere > + jclouds vSphere api > + Jclouds components to access an implementation of VMWare > vSphere. > + bundle > + > + > +https://localhost/sdk > +5.1 > + > +roo

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> + test-jar > + test > + > + > + org.apache.jclouds.driver > + jclouds-sshj > + ${project.version} > + test > + > + > + org.apache.jclouds.driver > + jclouds-log4j > + ${project.version} > + test > + > + Move this before `j

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> +package org.jclouds.vsphere; > + > +import com.vmware.vim25.FileFault; > +import com.vmware.vim25.InvalidDatastore; > +import com.vmware.vim25.RuntimeFault; > +import com.vmware.vim25.UserNotFound; > +import com.vmware.vim25.mo.Datacenter; > +import com.vmware.vim25.mo.Task; > + > +import java.i

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> +import com.vmware.vim25.UserNotFound; > +import com.vmware.vim25.mo.Datacenter; > +import com.vmware.vim25.mo.Task; > + > +import java.io.IOException; > +import java.rmi.RemoteException; > + > +/** > + * Date: 18/05/2014 5:31 PM > + * Package: org.jclouds.vsphere > + */ > +public interface IFile

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> + * Date: 18/05/2014 5:31 PM > + * Package: org.jclouds.vsphere > + */ > +public interface IFileManager { > + > + void uploadFile(String srcFilePath, String destDirectory) throws > IOException; > + > + void changeOwner(java.lang.String name, Datacenter datacenter, > java.lang.String owner)

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> +import com.vmware.vim25.mo.ScheduledTaskManager; > +import com.vmware.vim25.mo.ServerConnection; > +import com.vmware.vim25.mo.SessionManager; > +import com.vmware.vim25.mo.TaskManager; > +import com.vmware.vim25.mo.ViewManager; > +import com.vmware.vim25.mo.VirtualDiskManager; > +import org.jcl

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> + > +import com.vmware.vim25.FileFault; > +import com.vmware.vim25.InvalidDatastore; > +import com.vmware.vim25.RuntimeFault; > +import com.vmware.vim25.UserNotFound; > +import com.vmware.vim25.mo.Datacenter; > +import com.vmware.vim25.mo.Task; > + > +import java.io.IOException; > +import java.rm

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> + > + @Delegate > + ViewManager getViewManagerApi(); > + > + @Delegate > + VirtualDiskManager getVirtualDiskManagerApi(); > + > + @Delegate > + OptionManager getOptionManagerApi(); > + > + @Delegate > + Folder getRootFolder(); > + > + @Delegate > + ServerConnection getServerCo

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> @@ -0,0 +1,9 @@ > + Is this needed? I can't seem to find a `ConductorDeploymentTest` in this PR. If not needed, please remove. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/61/files#r14219247

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> @@ -0,0 +1,15 @@ > +# Copyright 2014 Cisco Systems, Inc Please remove this file, and instead create the context with the correct provider. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/61/files#r14219290

Re: [jclouds-labs] Add vSphere support (#61)

2014-06-25 Thread Andrew Phillips
> @@ -0,0 +1,363 @@ > +/* > +* Copyright 2014 Cisco Systems, Inc Please align this license header with the other files. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/61/files#r14219326

Re: [jclouds-karaf] Do not cache System.out in commands. (#47)

2014-06-25 Thread Andrew Phillips
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

Re: [jclouds-labs-aws] JCLOUDS-457: Added initiateJob (#25)

2014-06-25 Thread Andrew Phillips
> +import static com.google.common.base.Preconditions.checkNotNull; > + > +import org.jclouds.glacier.domain.JobRequest; > +import org.jclouds.http.HttpRequest; > +import org.jclouds.json.Json; > +import org.jclouds.rest.Binder; > + > +import com.google.inject.Inject; > + > +public class BindJobReq

Re: [jclouds-labs-aws] JCLOUDS-457: Added initiateJob (#25)

2014-06-25 Thread Andrew Phillips
> +import org.jclouds.glacier.domain.JobRequest; > +import org.jclouds.http.HttpRequest; > +import org.jclouds.json.Json; > +import org.jclouds.rest.Binder; > + > +import com.google.inject.Inject; > + > +public class BindJobRequestToJsonPayload implements Binder { > + @Inject > + private Json j

Re: [jclouds-labs-aws] JCLOUDS-457: Added initiateJob (#25)

2014-06-25 Thread Andrew Phillips
> +import org.jclouds.javax.annotation.Nullable; > + > +import com.google.common.base.Objects; > +import com.google.gson.annotations.SerializedName; > + > +public class InventoryRetrievalJobRequest extends JobRequest { > + private static final String TYPE = "inventory-retrieval"; > + > + @Seria

Re: [jclouds-labs-aws] JCLOUDS-457: Added initiateJob (#25)

2014-06-25 Thread Andrew Phillips
> + private String endDate; > + @SerializedName("Limit") > + private Integer limit; > + @SerializedName("Marker") > + private String marker; > + > + @ConstructorProperties({ "StartDate", "EndDate", "Limit", "Marker" }) > + private InventoryRetrievalParameters(@Nul

Re: [jclouds-labs-aws] JCLOUDS-457: Added initiateJob (#25)

2014-06-25 Thread Andrew Phillips
> +import org.jclouds.glacier.reference.GlacierHeaders; > +import org.jclouds.http.HttpException; > +import org.jclouds.http.HttpResponse; > + > +import com.google.common.base.Function; > + > +/** > + * Parses the jobId from the HttpResponse. > + */ > +public class ParseJobIdHeader implements Funct

Re: [jclouds-labs-aws] JCLOUDS-457: Added initiateJob (#25)

2014-06-25 Thread Andrew Phillips
Thanks, @rcoedo! A couple of comments. Also: do we need a test to ensure we behave correctly if a JobId is not sent in the response from the server, or is that really a situation that should never happen? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-

Re: [jclouds-karaf] Do not cache System.out in commands. (#47)

2014-06-25 Thread Andrew Phillips
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)

2014-06-25 Thread Andrew Phillips
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-labs-aws] JCLOUDS-457: Added initiateJob (#25)

2014-06-25 Thread Andrew Phillips
> + private String endDate; > + @SerializedName("Limit") > + private Integer limit; > + @SerializedName("Marker") > + private String marker; > + > + @ConstructorProperties({ "StartDate", "EndDate", "Limit", "Marker" }) > + private InventoryRetrievalParameters(@Nul

Re: [jclouds] JCLOUDS-610: reboot, suspend, and resumeNodesMatching methods gain functionality similar to that of destroyNodesMatching (#419)

2014-06-25 Thread Andrew Phillips
> change is backwards compatible in that it does not break/change existing > API's only what they return At the source level, it should be. As far as I can make out, though, at the binary level this counts as a backwards-incompatible change: "Changing the result type of a method, or replacing a

Re: [jclouds-labs-aws] Bug fix for ContentRange equals (#24)

2014-06-25 Thread Andrew Phillips
> Consequences of working late at night I guess Oh, I can imagine how it gone in the code. I'm more curious about how it slipped through the review ;-) Then again, I'm writing this at 2:35am... --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pul

[jclouds-karaf] Removing duplicate jclouds-log4j driver dependency (#49)

2014-06-25 Thread Andrew Phillips
Causes a warning in the build logs You can merge this Pull Request by running: git pull https://github.com/jclouds/jclouds-karaf remove-dup-driver Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-karaf/pull/49 -- Commit Summary -- * Removing dupli

Re: [jclouds-karaf] Do not cache System.out in commands. (#47)

2014-06-26 Thread Andrew Phillips
> 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] Removing duplicate jclouds-log4j driver dependency (#49)

2014-06-26 Thread Andrew Phillips
Merged to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds-karaf.git;a=commit;h=39e83ec8e78a8dac5c0ad7944a6e2e5e1e8cfc3a) and [1.7.x](https://git-wip-us.apache.org/repos/asf?p=jclouds-karaf.git;a=commit;h=404b331277458cd0a425ee76d8432b47e2a165a8) --- Reply to this email directly or vi

Re: [jclouds-karaf] Removing duplicate jclouds-log4j driver dependency (#49)

2014-06-26 Thread Andrew Phillips
Closed #49. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/49#event-135354594

Re: [jclouds-labs-openstack] Make sure metadata points to UK resources (#113)

2014-06-27 Thread Andrew Phillips
+1 - thanks, @everett-toews! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/113#issuecomment-47345903

Re: [jclouds] JCLOUDS-618: Allow servers without boot device in ElasticStack (#423)

2014-06-27 Thread Andrew Phillips
Code and tests look good to me. Does anything need to be made `@Nullable` as part of this change..? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/423#issuecomment-47373946

Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching throwing UnsupportedOpe... (#403)

2014-06-27 Thread Andrew Phillips
Merged to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;h=4d5f57a). Do we also want to backport this to 1.7.x? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/403#issuecomment-47374896

Re: [jclouds] JCLOUDS-618: Allow servers without boot device in ElasticStack (#423)

2014-06-27 Thread Andrew Phillips
Ah, yes, indeed...it was already possible for `imageId` to be null. +1 - looks good to me! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/423#issuecomment-47381409

Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching throwing UnsupportedOpe... (#403)

2014-06-28 Thread Andrew Phillips
> I definitely think this should be backported to 1.7.x @ccustine: Looks like [the patch](https://issues.apache.org/jira/secure/attachment/12652861/JCLOUDS-594.patch) does not directly apply :-( ``` error: patch failed: apis/openstack-nova/src/test/java/org/jclouds/openstack/nov a/v2_0/compute/

Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPA... (#425)

2014-06-30 Thread Andrew Phillips
> @@ -77,8 +77,13 @@ public > AllocateAndAddFloatingIpToNode(@Named(TIMEOUT_NODE_RUNNING) Predicate >FloatingIP ip = null; >try { > - logger.debug(">> allocating or reassigning floating ip for > node(%s)", node.getId()); > + > + logger.debug(">> allocating

Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPA... (#425)

2014-06-30 Thread Andrew Phillips
> @@ -94,6 +99,12 @@ public boolean apply(FloatingIP arg0) { > // try to prevent multiple parallel launches from choosing the same > ip. > Collections.shuffle(unassignedIps); > ip = Iterables.getLast(unassignedIps); > + > + //if we are still unable to

Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPA... (#425)

2014-06-30 Thread Andrew Phillips
> ip = floatingIpApi.create(); > + if(ip == null){ > + throw new InsufficientResourcesException(); > + } Please use a 3-space indent and spaces around the condition: ``` if (ip == null) { throw new... } ``` Also, if the IRE allows you to pass a message, pl

Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPA... (#425)

2014-06-30 Thread Andrew Phillips
Thanks, @cdancy! Some minor comments. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/425#issuecomment-47603668

Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPA... (#425)

2014-07-01 Thread Andrew Phillips
> @@ -77,8 +77,11 @@ public > AllocateAndAddFloatingIpToNode(@Named(TIMEOUT_NODE_RUNNING) Predicate >FloatingIP ip = null; >try { > - logger.debug(">> allocating or reassigning floating ip for > node(%s)", node.getId()); > + logger.debug(">> allocating or reassignin

Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPA... (#425)

2014-07-01 Thread Andrew Phillips
> @@ -94,6 +97,12 @@ public boolean apply(FloatingIP arg0) { > // try to prevent multiple parallel launches from choosing the same > ip. > Collections.shuffle(unassignedIps); > ip = Iterables.getLast(unassignedIps); > + > + //if we are still unable to

Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPA... (#425)

2014-07-01 Thread Andrew Phillips
As an aside: I know it's not related to this particular issue, but there's a bit of a nasty declaration of `unassignedIps` as an ArrayList (rather than simply a List) in the [affected class](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/no

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the L

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> @@ -45,6 +44,7 @@ > private final BlobToObject blob2Object; > private final MultipartUploadSlicingAlgorithm algorithm; > private final PayloadSlicer slicer; > + private final MultipartNamingStrategy namingStrategy = new > MultipartNamingStrategy(); Should this be injected by Guice

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> +String objectName = "file.txt"; > +long countBefore = blobStore.countBlobs(containerName); > + > +// we want 11 parts > +File fileToUpload = createFileExactly(PART_SIZE * 11, > objectName); > +addMultipartBlobToContainer(containerName,

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> +public void testMultipartChunkedFilenames() throws InterruptedException, > IOException { > +String containerName = getContainerName(); > +try { > +BlobStore blobStore = view.getBlobStore(); > +String objectName = "file.txt"; > +long countB

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> +File fileToUpload = createFileExactly(PART_SIZE * 11, > objectName); > +addMultipartBlobToContainer(containerName, objectName, > fileToUpload); > + > +// did we create enough parts? > +long countAfter = blobStore.countBlobs(containerName); > +

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> - .payload(fileToUpload) > - .build(); > - blobStore.putBlob(containerName, blob, PutOptions.Builder.multipart()); > + private File createFileExactly(long fileSize, String filename) throws > IOException { > + File fileToUpload = new File("target", filename); > +

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> - blobStore.createContainerInLocation(null, containerName); > - Blob blob = blobStore.blobBuilder(key) > - .payload(fileToUpload) > - .build(); > - blobStore.putBlob(containerName, blob, PutOptions.Builder.multipart()); > + private File createFileExactly(long file

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> +import org.testng.Assert; > +import org.testng.annotations.Test; > + > +@Test(testName = "MultipartNamingStrategyTest") > +public class MultipartNamingStrategyTest { > + > +@Test > +public void testGetPartNameFirstOneHundret() { > +final MultipartNamingStrategy strategy = new >

Re: [jclouds] Use the configured JCE provider in the Cipher payloads (#428)

2014-07-01 Thread Andrew Phillips
Code itself looks good. Quick question: would it make sense to maintain a backwards-compatible constructor? I.e. ``` public BaseCipherPayload(Payload delegate, Key key) { this(null, delegate, key); // pity there seems to be no way to get the "default crypto object"! } public BaseCipherPayload

Re: [jclouds] JCLOUDS-610: reboot, suspend, and resumeNodesMatching methods gain functionality similar to that of destroyNodesMatching (#419)

2014-07-01 Thread Andrew Phillips
Just to summarize this: are we thus OK with backporting this to 1.7.x? /cc @everett-toews @nacx? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/419#issuecomment-47672386

Re: [jclouds] JCLOUDS-610: reboot, suspend, and resumeNodesMatching methods gain functionality similar to that of destroyNodesMatching (#419)

2014-07-01 Thread Andrew Phillips
Merged to [1.7.x](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;h=7f28453) (and the [karaf fix](https://git-wip-us.apache.org/repos/asf?p=jclouds-karaf.git;a=commit;h=ca038e938501c242de53ffeb9409407c43a2109e), too). Thanks, @nacx! --- Reply to this email directly or view it on GitHub:

Re: [jclouds] Use the configured JCE provider in the Cipher payloads (#428)

2014-07-01 Thread Andrew Phillips
> That Crypto instance is the one that has the injected JCE provider, which is > the one users might > configure. We should enforce its use. Fine! Thanks for explaining. +1, then - looks good to me! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/4

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> +File fileToUpload = createFileExactly(PART_SIZE * 11, > objectName); > +addMultipartBlobToContainer(containerName, objectName, > fileToUpload); > + > +// did we create enough parts? > +long countAfter = blobStore.countBlobs(containerName); > +

Re: [jclouds-labs-aws] Use assertj fluent assertions where appropriate (#32)

2014-07-01 Thread Andrew Phillips
> } > > @Test(groups = { "integration", "live" }, dependsOnMethods = { > "testCreateVault" }) > public void testListMultipartUploadsWithEmptyList() throws Exception { > - assertEquals(api.listMultipartUploads(VAULT_NAME1).size(), 0); > + assertThat(api.listMultipartUploads(

Re: [jclouds-labs-aws] Use assertj fluent assertions where appropriate (#32)

2014-07-01 Thread Andrew Phillips
+1 - looks good to me, too. Thanks, @andrewgaul! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/32#issuecomment-47677326

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-01 Thread Andrew Phillips
> @@ -45,6 +44,7 @@ > private final BlobToObject blob2Object; > private final MultipartUploadSlicingAlgorithm algorithm; > private final PayloadSlicer slicer; > + private final MultipartNamingStrategy namingStrategy = new > MultipartNamingStrategy(); > Can you point me in the right

Re: [jclouds] refactor BaseComputeServiceLiveTest (#429)

2014-07-01 Thread Andrew Phillips
Pending PR builders +1 - looks good to me. Thanks, @andreaturli! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/429#issuecomment-47679961

Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching throwing UnsupportedOpe... (#403)

2014-07-01 Thread Andrew Phillips
> Added backport PR Thanks, @ccustine! Having a look at that now... --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/403#issuecomment-47680212

Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching throwing UnsupportedOperationException but call still succeeds (#424)

2014-07-01 Thread Andrew Phillips
> jclouds-java-7-pull-requests #1413 FAILURE [Spurious failure](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1413/console) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/424#issuecomment-47680289

Re: [jclouds] JCLOUDS-594: ComputeService.suspendNodesMatching throwing UnsupportedOperationException but call still succeeds (#424)

2014-07-01 Thread Andrew Phillips
There's a bunch of new [Checkstyle violations](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/942/violations/), only I think only [this one](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/942/org.apache.jclouds.api$openstack-nova/violations/file/src/test/java/org/jclouds/

Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPA... (#425)

2014-07-02 Thread Andrew Phillips
> Upon further thought I wonder if we should remove/deprecate the call to > "create()" altogether? I'm not familiar enough with this part of the code to comment - perhaps @zack-shoylev, @jdaggett or @everett-toews have some thoughts here? In the meantime, would you prefer to leave this PR open

Re: [jclouds] Added the CONTRIBUTING file (#430)

2014-07-02 Thread Andrew Phillips
> jclouds ยป jclouds #1315 FAILURE > jclouds-pull-requests #957 FAILURE > jclouds-java-7-pull-requests #1428 FAILURE Tsk. Tsk ;-) At least our CI system seems to be working reasonably, for now. Thanks for the fix, @nacx! --- Reply to this email directly or view it on GitHub: https://github.com/jc

Re: [jclouds] JCLOUDS-585 - Create HP Cloud block storage provider (#395)

2014-07-02 Thread Andrew Phillips
Thanks, @ccustine! Out of curiosity: do we have any live tests results for this? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/395#issuecomment-47816273

Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live tests (#31)

2014-07-02 Thread Andrew Phillips
> @@ -111,6 +111,7 @@ > > ${jclouds.blobstore.httpstream.md5} > > ${test.google-cloud-storage.identity} > > ${test.google-cloud-storage.credential} > + > ${test.google-cloud-storage.project-number} >

Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live tests (#31)

2014-07-02 Thread Andrew Phillips
> @@ -44,4 +45,11 @@ > @Delegate > @Path("") > BucketAccessControlsApi getBucketAccessControlsApi(); > + > + /** > +* Provides access to Bucket features > +*/ > + @Delegate > + @Path("") > + BucketApi getBucketsApi(); Should this be `getBucketApi()`? --- Reply to

Re: [jclouds-chef] JCLOUDS-617: Use the configured JCE provider in the Cipher payloads (#45)

2014-07-02 Thread Andrew Phillips
+1 - looks good to me. Thanks, @nacx! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-chef/pull/45#issuecomment-47839514

Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live tests (#31)

2014-07-02 Thread Andrew Phillips
> +import > org.jclouds.googlecloudstorage.domain.DomainResourceRefferences.Location; > +import > org.jclouds.googlecloudstorage.domain.DomainResourceRefferences.StorageClass; > +import org.jclouds.googlecloudstorage.domain.internal.BucketCors; > +import org.jclouds.googlecloudstorage.domain.inte

Re: [jclouds] JCLOUDS-619 (#427)

2014-07-02 Thread Andrew Phillips
Thanks for the updates, @mvrueden and the detailed review, @andrewgaul! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/427#issuecomment-47846953

Re: [jclouds] Handle short reads in BasePayloadSlicer (d43c3ea)

2014-07-02 Thread Andrew Phillips
Just to ensure I'm getting this right: we accumulate the value of successive reads in the offset until read == -1. Then we either return null if offset == 0, else we create a payload. I.e. is this equivalent to: ``` int offset = 0; while ((read = input.read(content, offset, readLen - offset) != -

Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live tests (#31)

2014-07-02 Thread Andrew Phillips
> +Long metageneration, Set acl, > Set defaultObjectAcl, > +Owner owner, Location location, Website website, Logging > logging, Versioning versioning, Set cors, > +BucketLifeCycle lifeCycle, StorageClass storageClass) { > + > + super(Kind.BUCKET, id, selfL

Re: [jclouds-labs-google] JCLOUD-458:Added Bucket Operation with live tests (#31)

2014-07-02 Thread Andrew Phillips
> + private final String name; > + private final Long projectNumber; > + private final Date timeCreated; > + private final Long metageneration; > + private final Set acl; > + private final Set defaultObjectAcl; > + private final Owner owner; > + private final Location location; > +

<    3   4   5   6   7   8   9   10   11   12   >