You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Zack Shoylev <no...@github.com> on 2013/08/26 18:54:17 UTC
[jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
https://issues.apache.org/jira/browse/JCLOUDS-215
This adds support for the scaling group api only.
You can merge this Pull Request by running:
git pull https://github.com/rackerlabs/jclouds-labs-openstack rackspace-autoscale
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/22
-- Commit Summary --
* Adds Otter (Rackspace Autoscale) to labs
-- File Changes --
M pom.xml (2)
A rackspace-autoscale-us/pom.xml (145)
A rackspace-autoscale-us/resources/META-INF/services/org.jclouds.apis.ApiMetadata (18)
A rackspace-autoscale-us/src/main/java/org/jclouds/rackspace/autoscale/us/v1/AutoscaleUSProviderMetadata.java (114)
A rackspace-autoscale-us/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (18)
A rackspace-autoscale-us/src/test/java/org/jclouds/rackspace/autoscale/us/v1/AutoscaleUSProviderMetadataExpectTest.java (64)
A rackspace-autoscale-us/src/test/java/org/jclouds/rackspace/autoscale/us/v1/AutoscaleUSProviderTest.java (32)
A rackspace-autoscale-us/src/test/java/org/jclouds/rackspace/autoscale/us/v1/features/AutoscaleUSGroupApiLiveTest.java (32)
A rackspace-autoscale-us/src/test/resources/access_rax_us.json (249)
A rackspace-autoscale-us/src/test/resources/logback.xml (69)
A rackspace-autoscale/pom.xml (134)
A rackspace-autoscale/resources/META-INF/services/org.jclouds.apis.ApiMetadata (18)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/AutoscaleApi.java (61)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/AutoscaleApiMetadata.java (96)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/binders/BindCreateGroupToJson.java (93)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/config/AutoscaleHttpApiModule.java (82)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/config/AutoscaleParserModule.java (32)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/Group.java (206)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/GroupConfiguration.java (221)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/GroupInstance.java (82)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/GroupState.java (145)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/LaunchConfiguration.java (339)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/LoadBalancer.java (129)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/Personality.java (125)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/ScalingPolicy.java (274)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/ScalingPolicyResponse.java (89)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/features/GroupApi.java (161)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/functions/ParseGroupResponse.java (151)
A rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/handlers/AutoscaleErrorHandler.java (67)
A rackspace-autoscale/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (18)
A rackspace-autoscale/src/test/java/org/jclouds/rackspace/autoscale/v1/features/GroupApiExpectTest.java (357)
A rackspace-autoscale/src/test/java/org/jclouds/rackspace/autoscale/v1/features/GroupApiLiveTest.java (217)
A rackspace-autoscale/src/test/java/org/jclouds/rackspace/autoscale/v1/internal/BaseAutoscaleApiExpectTest.java (27)
A rackspace-autoscale/src/test/java/org/jclouds/rackspace/autoscale/v1/internal/BaseAutoscaleApiLiveTest.java (43)
A rackspace-autoscale/src/test/java/org/jclouds/rackspace/autoscale/v1/internal/BaseAutoscaleExpectTest.java (82)
A rackspace-autoscale/src/test/resources/access_rax.json (249)
A rackspace-autoscale/src/test/resources/autoscale_groups_create_request.json (56)
A rackspace-autoscale/src/test/resources/autoscale_groups_create_response.json (72)
A rackspace-autoscale/src/test/resources/autoscale_groups_get_response.json (93)
A rackspace-autoscale/src/test/resources/autoscale_groups_list_response.json (33)
A rackspace-autoscale/src/test/resources/autoscale_groups_state_response.json (35)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/22.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/22.diff
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> + <version>1.7.0-SNAPSHOT</version>
> + </parent>
> +
> + <!-- TODO: when out of labs, switch to org.jclouds.api -->
> + <groupId>org.apache.jclouds.labs</groupId>
> + <artifactId>rackspace-autoscale-us</artifactId>
> + <version>1.7.0-SNAPSHOT</version>
> + <name>jclouds rackspace-autoscale api provider</name>
> + <description>jclouds components to access Rackspace Autoscale</description>
> + <packaging>bundle</packaging>
> +
> + <properties>
> + <!-- keystone endpoint -->
> + <test.rackspace-autoscale.endpoint>http://localhost:5000/v2.0/</test.rackspace-autoscale.endpoint>
> + <!-- keystone version -->
> + <test.rackspace-autoscale.api-version>1.0</test.rackspace-autoscale.api-version>
Endpoint and pachages are `2.0`. Should this be `2.0` by default too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6073689
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
+1. Thanks for the changes!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22#issuecomment-23564715
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.rest.annotations.Fallback;
> +import org.jclouds.rest.annotations.MapBinder;
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +import org.jclouds.rest.annotations.ResponseParser;
> +import org.jclouds.rest.annotations.SelectJson;
> +import org.jclouds.rest.annotations.SkipEncoding;
> +
> +import com.google.common.collect.FluentIterable;
> +
> +/**
> + * The API for controlling scaling groups.
> + * A scaling group is a high-level autoscaling concept that encompasses a group configuration, a launch configuration, and a set of scaling policies.
> + * @author Zack Shoylev
> + */
> +@SkipEncoding({'/', '='})
If I am not wrong, these characters are skipped by default. Does it work without this annotation?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6019175
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Zack Shoylev <no...@github.com>.
After double checking: you are right and both points need to be fixed. Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22#issuecomment-23531102
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Fallback(NullOnNotFoundOr404.class)
> + @MapBinder(BindCreateGroupToJson.class)
> + @ResponseParser(ParseGroupResponse.class)
> + Group create(@PayloadParam("groupConfiguration") GroupConfiguration groupConfiguration,
> + @PayloadParam("launchConfiguration") LaunchConfiguration launchConfiguration,
> + @PayloadParam("scalingPolicies") List<ScalingPolicy> scalingPolicies);
> +
> + /**
> + * This operation pauses the specified Autoscaling Group
> + *
> + * @param groupId The id for the specified Group.
> + * @return true if successful.
> + * @see GroupApi#resume(String)
> + */
> + @Named("Groups:pause/{id}")
This is the name of the property used to configure the timeout for this method, and AFAIK, the `{id}` is not substituted in this annotation. Do we want users to configure something like `jclouds.timeouts.Groups:pause/{id} = 1000`, or should the `{id}` be removed from the name?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6019318
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Zack Shoylev <no...@github.com>.
> +# contributor license agreements. See the NOTICE file distributed with
> +# this work for additional information regarding copyright ownership.
> +# The ASF licenses this file to You under the Apache License, Version 2.0
> +# (the "License"); you may not use this file except in compliance with
> +# the License. You may obtain a copy of the License at
> +#
> +# 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 License for the specific language governing permissions and
> +# limitations under the License.
> +#
> +
> +org.jclouds.openstack.trove.v1.TroveApiMetadata
Yeah, I think this should not have been submitted in the first place.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6023270
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #28](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/28/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22#issuecomment-23278088
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Andrew Phillips <no...@github.com>.
> +import com.google.common.base.Objects.ToStringHelper;
> +import com.google.common.collect.ImmutableList;
> +
> +/**
> + * Autoscale Group Instance (as in hardware instance). Part of the group state.
> + *
> + * @see GroupState#getGroupInstances()
> + * @author Zack Shoylev
> + */
> +public class GroupInstance {
> + private final String id;
> + private final ImmutableList<Link> links;
> +
> + @ConstructorProperties({ "id", "links" })
> + protected GroupInstance(String id, ImmutableList<Link> links) {
> + this.id = checkNotNull(id, "id should not be null");
Elsewhere the error message pattern is `x required`. Minor point, though ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6095411
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #332](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/332/) 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-labs-openstack/pull/22#issuecomment-23277956
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +# contributor license agreements. See the NOTICE file distributed with
> +# this work for additional information regarding copyright ownership.
> +# The ASF licenses this file to You under the Apache License, Version 2.0
> +# (the "License"); you may not use this file except in compliance with
> +# the License. You may obtain a copy of the License at
> +#
> +# 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 License for the specific language governing permissions and
> +# limitations under the License.
> +#
> +
> +org.jclouds.openstack.trove.v1.TroveApiMetadata
Just seen the path where this file is. The appropriate one is in `src/main/resources`. Should this file be removed?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6019444
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +import com.google.common.base.Objects.ToStringHelper;
> +import com.google.common.collect.ImmutableMap;
> +import com.google.common.collect.Maps;
> +
> +/**
> + * Autoscale GroupConfiguration.
> + *
> + * @see GroupApi#create(GroupConfiguration, LaunchConfiguration, java.util.List)
> + * @author Zack Shoylev
> + */
> +public class GroupConfiguration implements Comparable<GroupConfiguration>{
> + private final String name;
> + private final int cooldown;
> + private final int minEntities;
> + private final int maxEntities;
> + Map<String,String> metadata;
Also, does it make sense to change it to an `ImmutableMap`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6073764
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Andrew Phillips <no...@github.com>.
> + * @param links The links of this Group.
> + * @return The builder object.
> + * @see Group#getLinks()
> + */
> + public Builder links(List<Link> links) {
> + this.links = ImmutableList.copyOf(links);
> + return this;
> + }
> +
> + /**
> + * @param scalingPolicy The scaling policies list of this Group.
> + * @return The builder object.
> + * @see Group#getScalingPolicy()
> + */
> + public Builder scalingPolicy(List<ScalingPolicyResponse> scalingPolicy) {
> + this.scalingPolicy = ImmutableList.copyOf(scalingPolicy);
Do we need `checkNotNull` here and in similar places?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6095392
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Andrew Phillips <no...@github.com>.
> + <dependency>
> + <groupId>org.apache.jclouds.api</groupId>
> + <artifactId>openstack-keystone</artifactId>
> + <version>${project.parent.version}</version>
> + </dependency>
> + <dependency>
> + <groupId>org.apache.jclouds.api</groupId>
> + <artifactId>openstack-keystone</artifactId>
> + <version>${project.parent.version}</version>
> + <type>test-jar</type>
> + <scope>test</scope>
> + </dependency>
> + <dependency>
> + <groupId>org.apache.jclouds</groupId>
> + <artifactId>jclouds-core</artifactId>
> + <version>${project.parent.version}</version>
See comment about `jclouds.version` above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6095345
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Zack Shoylev <no...@github.com>.
> +import org.jclouds.rest.MapBinder;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +
> +import com.google.common.collect.ImmutableMap;
> +import com.google.common.collect.Lists;
> +import com.google.common.collect.Maps;
> +import com.google.inject.Inject;
> +
> +/**
> + * Decouple building the json object from the domain objects structure by using the binder
> + * @author Zack Shoylev
> + */
> +public class BindCreateGroupToJson implements MapBinder {
> +
> + @Inject
> + private BindToJsonPayload jsonBinder;
I see, it helps with failing-forward. I will refactor and see if there are issues. Other binders that currently use field injection might also have to be refactored to keep them consistent (eventually).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6042092
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Zack Shoylev <no...@github.com>.
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Fallback(NullOnNotFoundOr404.class)
> + @MapBinder(BindCreateGroupToJson.class)
> + @ResponseParser(ParseGroupResponse.class)
> + Group create(@PayloadParam("groupConfiguration") GroupConfiguration groupConfiguration,
> + @PayloadParam("launchConfiguration") LaunchConfiguration launchConfiguration,
> + @PayloadParam("scalingPolicies") List<ScalingPolicy> scalingPolicies);
> +
> + /**
> + * This operation pauses the specified Autoscaling Group
> + *
> + * @param groupId The id for the specified Group.
> + * @return true if successful.
> + * @see GroupApi#resume(String)
> + */
> + @Named("Groups:pause/{id}")
I will rename it..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6023520
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Andrew Phillips <no...@github.com>.
> I couldn't see any property jclouds.version (referenced in the OSGi imports) declared in the POMs...is that inherited
> from somewhere? If not, I suspect these bundles will be broken :-(
@iocanel: Any thoughts on this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22#issuecomment-27155138
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +/**
> + * Autoscale Group.
> + * This domain object contains information about an autoscaling group.
> + *
> + * @author Zack Shoylev
> + */
> +
> +public class Group {
> + private final String id;
> + private final ImmutableList<Link> links;
> + private final ImmutableList<ScalingPolicyResponse> scalingPolicy;
> + private final GroupConfiguration groupConfiguration;
> + private final LaunchConfiguration launchConfiguration;
> +
> + protected Group(String id, ImmutableList<Link> links, ImmutableList<ScalingPolicyResponse> scalingPolicy, GroupConfiguration groupConfiguration, LaunchConfiguration launchConfiguration) {
Ah, didn't see the response parser. It shouldn't be needed then.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6026213
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> + @Test
> + public void testListGroups() {
> + for (String zone : api.getConfiguredZones()) {
> + GroupApi groupApi = api.getGroupApiForZone(zone);
> + FluentIterable<GroupState> groupsList = groupApi.listGroupStates();
> + String groupId = created.get(zone).get(0).getId();
> + for( GroupState groupState : groupsList) {
> + if( groupId.equals(groupState.getId()) ) {
> + return;
> + }
> + }
> + fail("Could not find known groupID " + groupId);
> + }
> + }
> +
> + /* TODO: uncomment when implemented
Can this be uncommented?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6019388
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +import com.google.common.base.Objects.ToStringHelper;
> +import com.google.common.collect.ImmutableMap;
> +import com.google.common.collect.Maps;
> +
> +/**
> + * Autoscale GroupConfiguration.
> + *
> + * @see GroupApi#create(GroupConfiguration, LaunchConfiguration, java.util.List)
> + * @author Zack Shoylev
> + */
> +public class GroupConfiguration implements Comparable<GroupConfiguration>{
> + private final String name;
> + private final int cooldown;
> + private final int minEntities;
> + private final int maxEntities;
> + Map<String,String> metadata;
Make this one `private final` too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6073629
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.apis.ApiMetadata;
> +import org.jclouds.openstack.keystone.v2_0.config.KeystoneAuthenticationModule.ZoneModule;
> +import org.jclouds.providers.ProviderMetadata;
> +import org.jclouds.providers.internal.BaseProviderMetadata;
> +import org.jclouds.rackspace.autoscale.v1.AutoscaleApiMetadata;
> +import org.jclouds.rackspace.autoscale.v1.config.AutoscaleHttpApiModule;
> +import org.jclouds.rackspace.autoscale.v1.config.AutoscaleParserModule;
> +import org.jclouds.rackspace.cloudidentity.v2_0.config.CloudIdentityAuthenticationApiModule;
> +import org.jclouds.rackspace.cloudidentity.v2_0.config.CloudIdentityAuthenticationModule;
> +import org.jclouds.rackspace.cloudidentity.v2_0.config.CloudIdentityCredentialTypes;
> +
> +import com.google.common.collect.ImmutableSet;
> +import com.google.inject.Module;
> +
> +/**
> + * Implementation of {@link ApiMetadata} for Rackspace Autoscale API
Minor: change to `{@link ProviderMetadata}`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6018282
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Andrew Phillips <no...@github.com>.
> + authenticatedGET().endpoint(endpoint).build(),
> + HttpResponse.builder().statusCode(200).payload(payloadFromResource("/autoscale_groups_list_response.json")).build()
> + ).getGroupApiForZone("DFW");
> +
> + FluentIterable<GroupState> groupStates = api.listGroupStates();
> + assertEquals(groupStates.size(),2);
> +
> + assertEquals(groupStates.get(0).getGroupInstances().size(), 0);
> + assertEquals(groupStates.get(0).getActiveCapacity(), 0);
> + assertEquals(groupStates.get(0).getDesiredCapacity(), 0);
> + assertEquals(groupStates.get(0).getId(), "e41380ae-173c-4b40-848a-25c16d7fa83d");
> + assertEquals(groupStates.get(0).getLinks().size(), 1);
> + assertEquals(groupStates.get(0).getLinks().get(0).getHref().toString(), "https://dfw.autoscale.api.rackspacecloud.com/v1.0/676873/groups/e41380ae-173c-4b40-848a-25c16d7fa83d/");
> + assertEquals(groupStates.get(0).getLinks().get(0).getRelation(), Link.Relation.SELF);
> + assertEquals(groupStates.get(0).getPaused(), false);
> + assertEquals(groupStates.get(0).getPendingCapacity(), 0);
Wow...is there no way to do an "aggregate" check here, e.g. by constructing an "expected" object and doing `assertEquals(groupStates.get(0), expected);`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6095498
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #32](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/32/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22#issuecomment-23451092
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +# contributor license agreements. See the NOTICE file distributed with
> +# this work for additional information regarding copyright ownership.
> +# The ASF licenses this file to You under the Apache License, Version 2.0
> +# (the "License"); you may not use this file except in compliance with
> +# the License. You may obtain a copy of the License at
> +#
> +# 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 License for the specific language governing permissions and
> +# limitations under the License.
> +#
> +
> +org.jclouds.openstack.trove.v1.TroveApiMetadata
Should this be `org.jclouds.rackspace.autoscale.v1.AutoscaleApiMetadata`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6018248
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Zack Shoylev <no...@github.com>.
> +
> +/**
> + * Autoscale Group.
> + * This domain object contains information about an autoscaling group.
> + *
> + * @author Zack Shoylev
> + */
> +
> +public class Group {
> + private final String id;
> + private final ImmutableList<Link> links;
> + private final ImmutableList<ScalingPolicyResponse> scalingPolicy;
> + private final GroupConfiguration groupConfiguration;
> + private final LaunchConfiguration launchConfiguration;
> +
> + protected Group(String id, ImmutableList<Link> links, ImmutableList<ScalingPolicyResponse> scalingPolicy, GroupConfiguration groupConfiguration, LaunchConfiguration launchConfiguration) {
Technically, no (because of the response parser). It is not built directly by gson (which is why I think the contructor properties are needed?). Should I add those anyways?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6023388
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Zack Shoylev <no...@github.com>.
> +import org.jclouds.rest.MapBinder;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +
> +import com.google.common.collect.ImmutableMap;
> +import com.google.common.collect.Lists;
> +import com.google.common.collect.Maps;
> +import com.google.inject.Inject;
> +
> +/**
> + * Decouple building the json object from the domain objects structure by using the binder
> + * @author Zack Shoylev
> + */
> +public class BindCreateGroupToJson implements MapBinder {
> +
> + @Inject
> + private BindToJsonPayload jsonBinder;
I thought injected fields are not supposed to be final? From what I understand this is not a case of constructor injection.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6023349
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #354](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/354/) 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-labs-openstack/pull/22#issuecomment-23450950
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.rest.MapBinder;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +
> +import com.google.common.collect.ImmutableMap;
> +import com.google.common.collect.Lists;
> +import com.google.common.collect.Maps;
> +import com.google.inject.Inject;
> +
> +/**
> + * Decouple building the json object from the domain objects structure by using the binder
> + * @author Zack Shoylev
> + */
> +public class BindCreateGroupToJson implements MapBinder {
> +
> + @Inject
> + private BindToJsonPayload jsonBinder;
You're right. IMHO, though, using constructor injection has some benefits. using it + final fields forces the user to provide all dependencies, and if a new member is introduced, all the code that constructs the object just breaks. Unis tests are a common place where you may manually build the object. It is a good way to make sure all injected fields will be always present.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6026206
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #33](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/33/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22#issuecomment-23531744
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Zack Shoylev <no...@github.com>.
Usually I try to use java interfaces with immutable implementations. Anything a user accesses through a getter should be immutable.
About the enums, I will look at some other jclouds enums and try to keep it consistent with those.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22#issuecomment-23530067
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Andrew Phillips <no...@github.com>.
> + protected List<String> networks;
> + protected List<Personality> personalities;
> +
> + protected String serverName;
> + protected String serverImageRef;
> + protected String serverFlavorRef;
> + protected String serverDiskConfig;
> + protected ImmutableMap<String, String> serverMetadata;
> +
> + /**
> + * @param loadBalancers The load balancers of this LaunchConfiguration.
> + * @return The builder object.
> + * @see LaunchConfiguration#getLoadBalancers()
> + */
> + public Builder loadBalancers(List<LoadBalancer> loadBalancers) {
> + this.loadBalancers = loadBalancers;
This can thus be null...and will blow up in `copyOf`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6095431
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +# contributor license agreements. See the NOTICE file distributed with
> +# this work for additional information regarding copyright ownership.
> +# The ASF licenses this file to You under the Apache License, Version 2.0
> +# (the "License"); you may not use this file except in compliance with
> +# the License. You may obtain a copy of the License at
> +#
> +# 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 License for the specific language governing permissions and
> +# limitations under the License.
> +#
> +
> +org.jclouds.openstack.trove.v1.TroveApiMetadata
Same as above, should this be `org.jclouds.rackspace.autoscale.v1.AutoscaleApiMetadata`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6018349
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Zack Shoylev <no...@github.com>.
> + @Test
> + public void testListGroups() {
> + for (String zone : api.getConfiguredZones()) {
> + GroupApi groupApi = api.getGroupApiForZone(zone);
> + FluentIterable<GroupState> groupsList = groupApi.listGroupStates();
> + String groupId = created.get(zone).get(0).getId();
> + for( GroupState groupState : groupsList) {
> + if( groupId.equals(groupState.getId()) ) {
> + return;
> + }
> + }
> + fail("Could not find known groupID " + groupId);
> + }
> + }
> +
> + /* TODO: uncomment when implemented
The feature is not implemented service-side yet. Will be implemented soon.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6021305
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #30](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/30/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22#issuecomment-23384874
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Andrew Phillips <no...@github.com>.
> + <dependency>
> + <groupId>org.apache.jclouds.api</groupId>
> + <artifactId>openstack-keystone</artifactId>
> + <version>${project.parent.version}</version>
> + </dependency>
> + <dependency>
> + <groupId>org.apache.jclouds.api</groupId>
> + <artifactId>openstack-keystone</artifactId>
> + <version>${project.parent.version}</version>
> + <type>test-jar</type>
> + <scope>test</scope>
> + </dependency>
> + <dependency>
> + <groupId>org.apache.jclouds</groupId>
> + <artifactId>jclouds-core</artifactId>
> + <version>${project.parent.version}</version>
Should we be using `jclouds.version` for this and all jclouds core deps, to match the OSGi imports?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6095313
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
A couple considerations:
* The domain objects have a mix of mutable and immutable collections. Is that intentional, or could all them be changed to immutable ones?
* In jclouds we try to avoid using `null`, and one of the important points is to avoid using it as a return value in public methods to express absence ([null is evil](http://code.google.com/p/guava-libraries/wiki/UsingAndAvoidingNullExplained)!). Could the methods like the `getByValue` (and others, if any) used in the enums be refactored to return an `Optional`?
Apart from this last comments, this PR lgtm!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22#issuecomment-23525580
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +# contributor license agreements. See the NOTICE file distributed with
> +# this work for additional information regarding copyright ownership.
> +# The ASF licenses this file to You under the Apache License, Version 2.0
> +# (the "License"); you may not use this file except in compliance with
> +# the License. You may obtain a copy of the License at
> +#
> +# 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 License for the specific language governing permissions and
> +# limitations under the License.
> +#
> +
> +org.jclouds.openstack.trove.v1.TroveApiMetadata
Just seen the path where this file is. The appropriate one is in `src/main/resources`. Should this file be removed?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6019449
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> + <version>1.7.0-SNAPSHOT</version>
> + </parent>
> +
> + <!-- TODO: when out of labs, switch to org.jclouds.api -->
> + <groupId>org.apache.jclouds.labs</groupId>
> + <artifactId>rackspace-autoscale</artifactId>
> + <version>1.7.0-SNAPSHOT</version>
> + <name>jclouds rackspace-autoscale api</name>
> + <description>jclouds components to access Rackspace Autoscale</description>
> + <packaging>bundle</packaging>
> +
> + <properties>
> + <!-- keystone endpoint -->
> + <test.rackspace-autoscale.endpoint>http://localhost:5000/v2.0/</test.rackspace-autoscale.endpoint>
> + <!-- keystone version -->
> + <test.rackspace-autoscale.api-version>1.0</test.rackspace-autoscale.api-version>
Endpoint and pachages are `2.0`. Should this be `2.0` by default too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6073707
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Andrew Phillips <no...@github.com>.
> + Objects.equal(this.groupConfiguration, that.groupConfiguration) &&
> + Objects.equal(this.launchConfiguration, that.launchConfiguration);
> + }
> +
> + protected ToStringHelper string() {
> + return Objects.toStringHelper(this)
> + .add("id", id)
> + .add("links", links)
> + .add("scalingPolicy", scalingPolicy)
> + .add("groupConfiguration", groupConfiguration)
> + .add("launchConfiguration", "launchConfiguration");
> + }
> +
> + @Override
> + public String toString() {
> + return string().toString();
Any reason for this indirection, here and elsewhere?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6095383
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> + @ConstructorProperties({
> + "name", "cooldown", "minEntities", "maxEntities", "metadata"
> + })
> + protected GroupConfiguration(String name, int cooldown, int minEntities, int maxEntities, Map<String,String> metadata) {
> +
> + this.name = checkNotNull(name, "name required");
> + checkArgument(cooldown >= 0, "cooldown should be non-negative");
> + checkArgument(minEntities >= 0, "minEntities should be non-negative");
> + checkArgument(maxEntities >= 0, "maxEntities should be non-negative");
> + this.cooldown = cooldown;
> + this.minEntities = minEntities;
> + this.maxEntities = maxEntities;
> + this.metadata = metadata;
> + if(metadata == null) {
> + this.metadata = Maps.newHashMap();
> + }
Inline to avoid the reassignation? `this.metadata = metadata == null? Maps.newHashMap() : metadata;`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6073657
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.rest.MapBinder;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +
> +import com.google.common.collect.ImmutableMap;
> +import com.google.common.collect.Lists;
> +import com.google.common.collect.Maps;
> +import com.google.inject.Inject;
> +
> +/**
> + * Decouple building the json object from the domain objects structure by using the binder
> + * @author Zack Shoylev
> + */
> +public class BindCreateGroupToJson implements MapBinder {
> +
> + @Inject
> + private BindToJsonPayload jsonBinder;
Following Guice best practices, make it final?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6018596
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Andrew Phillips <no...@github.com>.
I couldn't see any property `jclouds.version` (referenced in the OSGi imports) declared in the POMs...is that inherited from somewhere? If not, I suspect these bundles will be broken :-(
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22#issuecomment-23587880
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +import com.google.common.base.Suppliers;
> +import com.google.common.collect.ImmutableMultimap;
> +import com.google.common.collect.Multimap;
> +import com.google.inject.Provides;
> +
> +/**
> + * Configures the Autoscale connection.
> + *
> + * @author Zack Shoylev
> + */
> +@ConfiguresHttpApi
> +public class AutoscaleHttpApiModule extends HttpApiModule<AutoscaleApi> {
> +
> + @Override
> + protected void configure() {
> + bind(DateAdapter.class).to(Iso8601DateAdapter.class);
This binding is redundant if the `AutoscaleParserModule` already adds it. Remove it from here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6018744
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> + * Autoscale Group State. Contains information about a scaling group.
> + *
> + * @see Group
> + * @see GroupApi#listGroupStates()
> + * @see GroupApi#getState(String)
> + * @author Zack Shoylev
> + */
> +public class GroupState implements Comparable<GroupState> {
> + private final String id;
> + private final List<Link> links;
> + private final int activeCapacity;
> + private final int pendingCapacity;
> + private final int desiredCapacity;
> + private final boolean paused;
> + @SerializedName("active")
> + private final List<GroupInstance> groupInstances;
Does it make sense to change this to an `ImmutableList`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6073749
Re: [jclouds-labs-openstack] Adds Otter (Rackspace Autoscale) to labs
(#22)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +/**
> + * Autoscale Group.
> + * This domain object contains information about an autoscaling group.
> + *
> + * @author Zack Shoylev
> + */
> +
> +public class Group {
> + private final String id;
> + private final ImmutableList<Link> links;
> + private final ImmutableList<ScalingPolicyResponse> scalingPolicy;
> + private final GroupConfiguration groupConfiguration;
> + private final LaunchConfiguration launchConfiguration;
> +
> + protected Group(String id, ImmutableList<Link> links, ImmutableList<ScalingPolicyResponse> scalingPolicy, GroupConfiguration groupConfiguration, LaunchConfiguration launchConfiguration) {
Does this need the `@ConstructorProperties` annotation?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/22/files#r6018831