You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by inbar stolberg <no...@github.com> on 2013/10/14 10:05:55 UTC
[jclouds] jclouds openstack list availability zones (#179)
list availability zones for Openstack + expect and live tests
You can merge this Pull Request by running:
git pull https://github.com/inbarsto/jclouds-1 listZones
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/179
-- Commit Summary --
* jclouds openstack list availability zones
-- File Changes --
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/NovaApi.java (14)
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/NovaAsyncApi.java (13)
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/config/NovaRestClientModule.java (33)
A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/zonescoped/AvailabilityZone.java (120)
A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/zonescoped/ZoneState.java (105)
A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/extensions/AvailabilityZoneAPI.java (44)
A apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/extensions/AvailabilityZoneAsyncApi.java (62)
A apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/extensions/AvailabilityZoneApiExpectTest.java (63)
A apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/extensions/AvailabilityZonesApiLiveTest.java (46)
A apis/openstack-nova/src/test/resources/listAvailabilityZones.json (11)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/179.patch
https://github.com/jclouds/jclouds/pull/179.diff
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> +/**
> + * @author Inbar Stolberg
> + */
> +public class ZoneState {
> +
> + private final Boolean isAvailable;
> +
> + @ConstructorProperties({
> + "available"
> + })
> + protected ZoneState(Boolean isAvailable) {
> + this.isAvailable = isAvailable;
> + }
> +
> + public Boolean isAvailable() {
> + return this.isAvailable;
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992686
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
from what i got from @everett-toews (in a different PR) if the parameter and the value in the @ConstructorProperties are the same then i can remove @ConstructorProperties entirely (just tested in live test to make sure and this seems to work fine..)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6997584
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #529](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/529/) UNSTABLE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26412060
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
If I am not wrong, the current values for the `@ConstructorProperties` are the right ones. At least the ones jclouds expects. Those values are used to deserialize the response. The [GsonModule](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/json/config/GsonModule.java#L123-L124) configures a [specific strategy](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/json/internal/NamingStrategies.java#L200-L250) to deserialize responses when the constructor has arguments.
So the values in the annotation are the right ones to get the response properly deserialized, but perhaps the name of the field and the constructor parameters could be changed to keep a more purist naming convention.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6997339
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
No, I mean that the name of the actual constructor parameter is `name`, but here in the annotation you're using `zoneName`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6996474
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
> np i will change variable name from name to zoneName (and will remove the constructor since there is no need for it if the parameters
> are 1:1 as in openstack
Before we do this, might be good to get something like @everett-toews or @nacx to look at this, since they know more about what `@ConstructorProperties` actually does here (after all, the annotation may be being used for some serialization stuff) than I do ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6997171
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
Nice property names can be used without problem. The only thing to take into account is that you need to tell Gson how to deserialize and serialize (although it might not be needed here I'd also configure that, to make the whole thing consistent) the entity into a JSON object. The first is done with the `@ConstructorProperties` annotation, and the latter with the `@SerializedName` one.
You could have something like:
```java
public class AvailabilityZone {
@SerializedName("zoneName")
private final String name;
private final ZoneState zoneState;
@ConstructorProperties({"zoneName" , "zoneState"})
protected AvailabilityZone(String name, ZoneState zoneState) {
...
}
public String getName() {
return name;
}
```
And keep the nice property names and getter/setters.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6998185
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
> what is the sentence regarding the @ConstructorProperties ?
As far as I understand the responses from Everett and Ignasi, we can either:
* use the JSON field names (zoneName and zoneState, I guess) as the constructor property names and then drop the annotation **or**
* use "nice" constructor property names (name and state) and keep the annotation as-is to help the deserializer. Not sure if that blows up GSON because the getters and setters would then be `getName` and `getState` rather than `getZoneName` and `getZoneState`
I'm wondering whether there's a third option
* use the JSON field names as constructor property names but use "nice" names for the instance variables. This would suffer from the same problem as option 2 above
The latter option would be my preferred choice, if it works, otherwise we're left with option 1.
@nacx Any comment on that? And thanks to you and @everett-toews for explaining - learned something new again today!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6997934
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #509](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/509/) 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/179#issuecomment-26243245
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> +import com.google.common.collect.FluentIterable;
> +import org.jclouds.openstack.nova.v2_0.domain.zonescoped.AvailabilityZone;
> +import org.jclouds.openstack.v2_0.ServiceType;
> +import org.jclouds.openstack.v2_0.services.Extension;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +@Beta
> +@Extension(of = ServiceType.COMPUTE, namespace = ExtensionNamespaces.ADMIN_ACTIONS)
> +public interface AvailabilityZoneAPI {
> +
> + /**
> + * List all availability zones
> + *
> + * @return all availability zones
I think the `@return` line is enough here - the Javadoc comment just duplicates it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973591
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class ZoneState {
> +
> + private final Boolean isAvailable;
> +
> + @ConstructorProperties({
> + "available"
> + })
to keep the java convention of boolean with the name of openstack i must use the @ConstructorProperties with "available"
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992677
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #763](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/763/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26241890
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
https://issues.apache.org/jira/browse/JCLOUDS-353
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26666699
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> +/**
> + * @author Inbar Stolberg
> + */
> +public class ZoneState {
> +
> + private final Boolean isAvailable;
> +
> + @ConstructorProperties({
> + "available"
> + })
> + protected ZoneState(Boolean isAvailable) {
> + this.isAvailable = isAvailable;
> + }
> +
> + public Boolean isAvailable() {
> + return this.isAvailable;
See comment above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973560
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
squashed and ready to go :) , added a jira ticket see comment above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26669489
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> +
> +/**
> + * @author inbar stolberg
> + */
> +@Test(groups = "live", testName = "AvailabilityZonesApiLiveTest")
> +public class AvailabilityZonesApiLiveTest extends BaseNovaApiLiveTest {
> +
> + @Test
> + public void testListAvailabilityZones() throws Exception {
> +
> + AvailabilityZoneAPI availabilityZoneApi = api.getAvailabilityZoneApi("RegionOne");
> + FluentIterable<? extends AvailabilityZone> zones = availabilityZoneApi.list();
> +
> + for (AvailabilityZone zone : zones) {
> + Assert.assertNotNull(zone.getName());
> + Assert.assertTrue(zone.getZoneState().isAvailable());
Static import to remove the `Assert.`, and add a message for `assertTrue`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973714
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
Just to be more complete, the `@SerializedName` annotation is used for both serialization and deserialization, but that would require a default constructor. As we have an immutable model and have a constructor that requires all the parameters, we need to somehow tell Gson how to map the fields in the JSON object to the arguments of the constructor, and that is what jclouds does with the `@ConstructorProperties` annotation and the strategy mentioned in the previous comment.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6998272
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.openstack.nova.v2_0.domain.zonescoped.AvailabilityZone;
> +import org.jclouds.openstack.v2_0.ServiceType;
> +import org.jclouds.openstack.v2_0.services.Extension;
> +import org.jclouds.rest.annotations.Fallback;
> +import org.jclouds.rest.annotations.RequestFilters;
> +import org.jclouds.rest.annotations.SelectJson;
> +
> +import javax.ws.rs.Consumes;
> +import javax.ws.rs.GET;
> +import javax.ws.rs.Path;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +
[minor] Remove blank line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973620
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> +import com.google.common.collect.FluentIterable;
> +import org.jclouds.openstack.nova.v2_0.domain.zonescoped.AvailabilityZone;
> +import org.jclouds.openstack.v2_0.ServiceType;
> +import org.jclouds.openstack.v2_0.services.Extension;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +@Beta
> +@Extension(of = ServiceType.COMPUTE, namespace = ExtensionNamespaces.ADMIN_ACTIONS)
> +public interface AvailabilityZoneAPI {
> +
> + /**
> + * List all availability zones
> + *
> + * @return all availability zones
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992693
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class ZoneState {
> +
> + private final Boolean isAvailable;
> +
> + @ConstructorProperties({
> + "available"
> + })
Wrong name?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973550
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #782](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/782/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26409135
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Everett Toews <no...@github.com>.
I ran the live test successfully. Just a couple of things left to do.
- [ ] Create a JIRA issue for this and put the link in the PR description
- [ ] Squash down the commits
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26451524
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + .addHeader("X-Auth-Token", authToken).build();
> +
> + HttpResponse listResponse = HttpResponse.builder().statusCode(200)
> + .payload(payloadFromResource("/listAvailabilityZones.json")).build();
> +
> + NovaApi availabilityZonesApi = requestsSendResponses(keystoneAuthWithUsernameAndPasswordAndTenantName,
> + responseWithKeystoneAccess, extensionsOfNovaRequest, extensionsOfNovaResponse, list, listResponse);
> +
> + assertEquals(availabilityZonesApi.getConfiguredZones(), ImmutableSet.of("az-1.region-a.geo-1", "az-2.region-a.geo-1", "az-3.region-a.geo-1"));
> +
> + FluentIterable<? extends AvailabilityZone> zones = availabilityZonesApi.getAvailabilityZoneApi("az-1.region-a.geo-1").list();
> +
> + Optional<? extends AvailabilityZone> zone = zones.first();
> +
> + assertTrue(zone.isPresent());
> + Assert.assertTrue(zone.get().getName().equals("nova"));
Could/should this be case-insensitive?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973687
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> +import org.jclouds.openstack.nova.v2_0.domain.zonescoped.AvailabilityZone;
> +import org.jclouds.openstack.v2_0.ServiceType;
> +import org.jclouds.openstack.v2_0.services.Extension;
> +import org.jclouds.rest.annotations.Fallback;
> +import org.jclouds.rest.annotations.RequestFilters;
> +import org.jclouds.rest.annotations.SelectJson;
> +
> +import javax.ws.rs.Consumes;
> +import javax.ws.rs.GET;
> +import javax.ws.rs.Path;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992705
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
so i changed the parameter name from isAvailable to available, what is the sentence regarding the @ConstructorProperties ?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6997620
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Everett Toews <no...@github.com>.
Merged.
Go ahead and close out the JIRA issue.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26719852
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> +
> + HttpResponse listResponse = HttpResponse.builder().statusCode(200)
> + .payload(payloadFromResource("/listAvailabilityZones.json")).build();
> +
> + NovaApi availabilityZonesApi = requestsSendResponses(keystoneAuthWithUsernameAndPasswordAndTenantName,
> + responseWithKeystoneAccess, extensionsOfNovaRequest, extensionsOfNovaResponse, list, listResponse);
> +
> + assertEquals(availabilityZonesApi.getConfiguredZones(), ImmutableSet.of("az-1.region-a.geo-1", "az-2.region-a.geo-1", "az-3.region-a.geo-1"));
> +
> + FluentIterable<? extends AvailabilityZone> zones = availabilityZonesApi.getAvailabilityZoneApi("az-1.region-a.geo-1").list();
> +
> + Optional<? extends AvailabilityZone> zone = zones.first();
> +
> + assertTrue(zone.isPresent());
> + Assert.assertTrue(zone.get().getName().equals("nova"));
> + Assert.assertTrue(zone.get().getZoneState().isAvailable());
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992740
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #309](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/309/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26311752
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
oh :) np i will change variable name from name to zoneName (and will remove the constructor since there is no need for it if the parameters are 1:1 as in openstack
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6996531
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + @Override
> + public int hashCode() {
> + return Objects.hashCode(name, zoneState);
> + }
> +
> + @Override
> + public boolean equals(Object obj) {
> + if (this != obj) return false;
> + if (obj == null || getClass() != obj.getClass()) return false;
> + AvailabilityZone that = AvailabilityZone.class.cast(obj);
> + return Objects.equal(this.name, that.name) && Objects.equal(this.zoneState, that.zoneState);
> + }
> +
> + protected Objects.ToStringHelper string() {
> + return Objects.toStringHelper(this)
> + .add("zoneName", name)
`name` or `zoneName`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973533
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class ZoneState {
> +
> + private final boolean isAvailable;
> +
> + @ConstructorProperties({
> + "available"
> + })
> + protected ZoneState(Boolean isAvailable) {
> + this.isAvailable = isAvailable;
> + }
> +
> + public boolean isAvailable() {
To be compatible with JavaBeans conventions (jclouds is not terribly strict about that, though), the property itself should then be called `available`, not `isAvailable`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6996603
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #771](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/771/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26311705
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #799](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/799/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26669754
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> @@ -70,6 +71,14 @@
> Set<String> getConfiguredZones();
>
> /**
> + * Provides asynchronous access to availability zone features
> + */
> + @Delegate
> + AvailabilityZoneAPI getAvailabilityZoneApi(
> + @EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
[minor] Remove one blank line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973463
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
@nacx: Thanks again for the explanation!
@inbarsto: In that case, my personal preference would be to follow Ignasi's example and also rename `zoneState` to `state`. What do you think?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6998539
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #301](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/301/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26241876
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> @@ -70,6 +71,14 @@
> Set<String> getConfiguredZones();
>
> /**
> + * Provides asynchronous access to availability zone features
> + */
> + @Delegate
> + AvailabilityZoneAPI getAvailabilityZoneApi(
> + @EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992636
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> + * "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.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class ZoneState {
> +
> + private final Boolean isAvailable;
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992684
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> +import javax.ws.rs.Path;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +
> +@Beta
> +@Extension(of = ServiceType.COMPUTE, namespace = ExtensionNamespaces.ADMIN_ACTIONS)
> +@RequestFilters(AuthenticateRequest.class)
> +public interface AvailabilityZoneAsyncApi {
> +
> + /**
> + * List all availability zones
> + *
> + * @return all availability zones
Again, I think the `@return` line should be sufficient
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973630
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
> + })
> + protected AvailabilityZone(String name, ZoneState zoneState) {
> + this.name = name;
> + this.zoneState = zoneState;
> + }
> +
> + public String getName() {
> + return this.name;
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992637
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
> + })
> + protected AvailabilityZone(String name, ZoneState zoneState) {
> + this.name = name;
> + this.zoneState = zoneState;
> + }
> +
> + public String getName() {
> + return this.name;
[minor] I think we use `return name` (i.e. no `this`) elsewhere in jclouds?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973514
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #786](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/786/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26412530
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
nope the names are correct this is for the response from openstack (i noticed that the convention is than if one parameter is different by name than i should add all the parameters to @ConstructorProperties
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992623
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> +
> +/**
> + * @author inbar stolberg
> + */
> +@Test(groups = "live", testName = "AvailabilityZonesApiLiveTest")
> +public class AvailabilityZonesApiLiveTest extends BaseNovaApiLiveTest {
> +
> + @Test
> + public void testListAvailabilityZones() throws Exception {
> +
> + AvailabilityZoneAPI availabilityZoneApi = api.getAvailabilityZoneApi("RegionOne");
> + FluentIterable<? extends AvailabilityZone> zones = availabilityZoneApi.list();
> +
> + for (AvailabilityZone zone : zones) {
> + assertNotNull(zone.getName());
> + assertTrue(zone.getZoneState().isAvailable(), "zone: " + zone.getName() + "is not available.");
[minor] Probably need a space before the "is"?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6996562
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
Commit [on master](https://git-wip-us.apache.org/repos/asf?p=incubator-jclouds.git;a=commit;h=725b7c5c2c3c0751f67eb2256b4ea8b8114de155). Thanks, @everett-toews and @inbarsto!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26735756
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #777](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/777/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26394574
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #336](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/336/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26669741
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> +
> +/**
> + * @author inbar stolberg
> + */
> +@Test(groups = "live", testName = "AvailabilityZonesApiLiveTest")
> +public class AvailabilityZonesApiLiveTest extends BaseNovaApiLiveTest {
> +
> + @Test
> + public void testListAvailabilityZones() throws Exception {
> +
> + AvailabilityZoneAPI availabilityZoneApi = api.getAvailabilityZoneApi("RegionOne");
> + FluentIterable<? extends AvailabilityZone> zones = availabilityZoneApi.list();
> +
> + for (AvailabilityZone zone : zones) {
> + Assert.assertNotNull(zone.getName());
> + Assert.assertTrue(zone.getZoneState().isAvailable());
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992773
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> +import javax.ws.rs.Path;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +
> +@Beta
> +@Extension(of = ServiceType.COMPUTE, namespace = ExtensionNamespaces.ADMIN_ACTIONS)
> +@RequestFilters(AuthenticateRequest.class)
> +public interface AvailabilityZoneAsyncApi {
> +
> + /**
> + * List all availability zones
> + *
> + * @return all availability zones
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992708
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class AvailabilityZone {
> +
> + private final String name;
> + private final ZoneState zoneState;
> +
> + @ConstructorProperties({
> + "zoneName" , "zoneState"
Wrong names here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973479
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class ZoneState {
> +
> + private final boolean isAvailable;
> +
> + @ConstructorProperties({
> + "available"
> + })
Same comment as above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6996537
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> + * "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.
> + */
> +package org.jclouds.openstack.nova.v2_0.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class ZoneState {
> +
> + private final Boolean isAvailable;
`boolean`? Or can this be `null`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973555
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by Andrew Phillips <no...@github.com>.
> +
> + HttpResponse listResponse = HttpResponse.builder().statusCode(200)
> + .payload(payloadFromResource("/listAvailabilityZones.json")).build();
> +
> + NovaApi availabilityZonesApi = requestsSendResponses(keystoneAuthWithUsernameAndPasswordAndTenantName,
> + responseWithKeystoneAccess, extensionsOfNovaRequest, extensionsOfNovaResponse, list, listResponse);
> +
> + assertEquals(availabilityZonesApi.getConfiguredZones(), ImmutableSet.of("az-1.region-a.geo-1", "az-2.region-a.geo-1", "az-3.region-a.geo-1"));
> +
> + FluentIterable<? extends AvailabilityZone> zones = availabilityZonesApi.getAvailabilityZoneApi("az-1.region-a.geo-1").list();
> +
> + Optional<? extends AvailabilityZone> zone = zones.first();
> +
> + assertTrue(zone.isPresent());
> + Assert.assertTrue(zone.get().getName().equals("nova"));
> + Assert.assertTrue(zone.get().getZoneState().isAvailable());
Remove the `Assert.` bit? And please add a message to the `assertTrue` that explains what is expected - this makes it easier to understand test failures.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6973678
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #324](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/324/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26412525
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #315](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/315/) 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/179#issuecomment-26394562
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #320](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/320/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179#issuecomment-26409117
Re: [jclouds] jclouds openstack list availability zones (#179)
Posted by inbar stolberg <no...@github.com>.
> + @Override
> + public int hashCode() {
> + return Objects.hashCode(name, zoneState);
> + }
> +
> + @Override
> + public boolean equals(Object obj) {
> + if (this != obj) return false;
> + if (obj == null || getClass() != obj.getClass()) return false;
> + AvailabilityZone that = AvailabilityZone.class.cast(obj);
> + return Objects.equal(this.name, that.name) && Objects.equal(this.zoneState, that.zoneState);
> + }
> +
> + protected Objects.ToStringHelper string() {
> + return Objects.toStringHelper(this)
> + .add("zoneName", name)
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/179/files#r6992660