You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by inbar stolberg <no...@github.com> on 2014/10/08 07:26:52 UTC
[jclouds] cinder availability zones api + list call implemented
(#560)
availability zones api for openstack cinder and list availability zones for cinder
You can merge this Pull Request by running:
git pull https://github.com/inbarsto/cloudband-jclouds cinderAZ
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/560
-- Commit Summary --
* cinder availability zones api + list call implemented
-- File Changes --
M apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/CinderApi.java (7)
A apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/domain/zonescoped/AvailabilityZone.java (119)
A apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/domain/zonescoped/ZoneState.java (105)
A apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/extensions/AvailabilityZoneApi.java (58)
A apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/extensions/ExtensionNamespaces.java (30)
A apis/openstack-cinder/src/test/java/org/jclouds/openstack/cinder/v1/features/AvailabilityZoneApiExpectTest.java (59)
A apis/openstack-cinder/src/test/java/org/jclouds/openstack/cinder/v1/features/AvailabilityZoneApiLiveTest.java (58)
A apis/openstack-cinder/src/test/resources/availability_zones_list.json (8)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/560.patch
https://github.com/jclouds/jclouds/pull/560.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
>grin. Hopefully, an experience that changes things for the better - not just for you, but for all contributors. So we all have something to be grateful to you for ;-)
you guys are awesome jclouds community is gr8 :)
it is for 1.8 i'll be happy if it will land there and in master as well
if you want me to create a different PR for master thats ok too...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58813217
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
> + public abstract static class Builder<T extends Builder<T>> {
> + protected abstract T self();
> +
> + protected ZoneState state;
> + protected String name;
> +
> + /**
> + * @see AvailabilityZone#getName()
> + */
> + public T name(String name) {
> + this.name = name;
> + return self();
> + }
> +
> + /**
> + * @see org.jclouds.openstack.cinder.v1.domain.zonescoped.AvailabilityZone#getState()
True
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18599332
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1261](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1261/) 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/560#issuecomment-58334574
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> +import org.jclouds.openstack.keystone.v2_0.filters.AuthenticateRequest;
> +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;
> +
> +/**
> + * api for cinder availability zones
> + */
> +
extra space nit
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18591901
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by cloudband <no...@github.com>.
thank you for the review :) fixing now
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58324424
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1765](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1765/) 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/560#issuecomment-58555110
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
> + private static class ConcreteBuilder extends Builder<ConcreteBuilder> {
> + @Override
> + protected ConcreteBuilder self() {
> + return this;
> + }
> + }
> +
> + private final String name;
> + private final ZoneState state;
> +
> + @ConstructorProperties({
> + "zoneName", "zoneState"
> + })
> + protected AvailabilityZone(String name, ZoneState zoneState) {
> + this.name = name;
> + this.state = zoneState;
Are rhis fields mandatory or nullable?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18568042
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
> @@ -79,4 +80,10 @@ SnapshotApi getSnapshotApiForZone(
> QuotaApi getQuotaApi(
> @EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
>
> + /**
> + * provides synchronous access to availability zone features
> + */
> + @Delegate
> + AvailabilityZoneApi getAvailabilityZoneApi(
> + @EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
leaving for convention sake hope its ok.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18658452
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #168](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/168/) 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/560#issuecomment-58315164
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Andrew Phillips <no...@github.com>.
> it was hmm... an experience
*grin*. Hopefully, an experience that changes things for the better - not just for you, but for all contributors. So we **all** have something to be grateful to you for ;-)
This patch is against 1.8.x - is that intentional? If so, we can merge it after 1.8.1 is out. If this also needs to be applied to master, I'll try to fix it (it's not merging cleanly on master right now) and open a PR for that.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58811659
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1755](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1755/) 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/560#issuecomment-58346752
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1265](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1265/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58346183
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Andrew Phillips <no...@github.com>.
@inbarsto Just a logic question about a domain object accessor. No need for another commit on your side, just to confirm that things are indeed correct as-is. I'll take care of @adriancole's comment during the merge.
Thanks for living through this one!!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58805102
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
i think i fixed it all accept for the concrete builder i can use an example...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58328772
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Andrew Phillips <no...@github.com>.
> +
> + private static class ConcreteBuilder extends Builder<ConcreteBuilder> {
> + @Override
> + protected ConcreteBuilder self() {
> + return this;
> + }
> + }
> +
> + private final Boolean available;
> +
> + @ConstructorProperties({"available"})
> + protected ZoneState(Boolean available) {
> + this.available = available;
> + }
> +
> + public boolean available() {
@inbarsto Does serialization/deserialization work with this, or does it need to be `isAvailable()` or `getAvailable()` or so?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18748193
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #180](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/180/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58546859
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1263](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1263/) 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/560#issuecomment-58338905
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Adrian Cole <no...@github.com>.
> @@ -0,0 +1,8 @@
> +{
> + "availabilityZoneInfo": [{
> + "zoneState": {
> + "available": true
> + },
> + "zoneName": "nova"
> + }]
> +}
nit eol before eof.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18587967
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> +@Extension(of = ServiceType.BLOCK_STORAGE, namespace = ExtensionNamespaces.ADMIN_ACTIONS)
> +@RequestFilters(AuthenticateRequest.class)
> +public interface AvailabilityZoneApi {
> +
> +
> + /**
> + * List all availability zones
> + *
> + * @return all availability zones
> + */
> + @GET
> + @Path("/os-availability-zone")
> + @SelectJson("availabilityZoneInfo")
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Fallback(Fallbacks.EmptyFluentIterableOnNotFoundOr404.class)
> + FluentIterable<? extends AvailabilityZone> list();
```java
FluentIterable<AvailabilityZone> list();
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18597344
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by cloudband <no...@github.com>.
> +
> +/**
> + * availability zone for cinder
> + */
> +public class AvailabilityZone {
> +
> + public static Builder<?> builder() {
> + return new ConcreteBuilder();
> + }
> +
> + public Builder<?> toBuilder() {
> + return new ConcreteBuilder().fromAvailabilityZone(this);
> + }
> +
> + public static abstract class Builder<T extends Builder<T>> {
> + protected abstract T self();
you mean to remove the concrete builder? or do i need to remove the abstract builder as well?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18568110
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1756](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1756/) 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/560#issuecomment-58352046
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
> + private static class ConcreteBuilder extends Builder<ConcreteBuilder> {
> + @Override
> + protected ConcreteBuilder self() {
> + return this;
> + }
> + }
> +
> + private final String name;
> + private final ZoneState state;
> +
> + @ConstructorProperties({
> + "zoneName", "zoneState"
> + })
> + protected AvailabilityZone(String name, ZoneState zoneState) {
> + this.name = name;
> + this.state = zoneState;
Add the corresponding `checkNotNull` checks then, to fail fast if one of the fields is missing?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18568465
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +/**
> + * availability zone for cinder
> + */
> +public class AvailabilityZone {
> +
> + public static Builder<?> builder() {
> + return new ConcreteBuilder();
> + }
> +
> + public Builder<?> toBuilder() {
> + return new ConcreteBuilder().fromAvailabilityZone(this);
> + }
> +
> + public static abstract class Builder<T extends Builder<T>> {
> + protected abstract T self();
There is no need for the abstract/concrete builders if the class is not meant to be subclassed. Leave just one Builder object.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18567986
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1751](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1751/) 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/560#issuecomment-58327021
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> + * availability zone for cinder
> + */
> +public class AvailabilityZone {
> +
> + public static Builder<?> builder() {
> + return new ConcreteBuilder();
> + }
> +
> + public Builder<?> toBuilder() {
> + return new ConcreteBuilder().fromAvailabilityZone(this);
> + }
> +
> + public abstract static class Builder<T extends Builder<T>> {
> + protected abstract T self();
> +
> + protected ZoneState state;
Interesting. I would think that the `AvailabilityZone` is either enabled or disabled. `ZoneState` only provides a boolean field "available", so that leads me to believe that this might be extended in the future.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18597924
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #174](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/174/) 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/560#issuecomment-58338664
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> +public class ZoneState {
> +
> + public static Builder<?> builder() {
> + return new ConcreteBuilder();
> + }
> +
> + public Builder<?> toBuilder() {
> + return new ConcreteBuilder().fromZoneState(this);
> + }
> +
> + public abstract static class Builder<T extends Builder<T>> {
> + protected abstract T self();
> +
> + protected boolean available;
> +
> +
Remove line
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18597203
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> + public T fromZoneState(ZoneState in) {
> + return this
> + .available(in.available());
> + }
> + }
> +
> + private static class ConcreteBuilder extends Builder<ConcreteBuilder> {
> + @Override
> + protected ConcreteBuilder self() {
> + return this;
> + }
> + }
> +
> + private final Boolean available;
> +
> + @ConstructorProperties({
```java
@ConstructorProperties({"available"})
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18597244
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
> +public class AvailabilityZoneApiExpectTest extends BaseCinderApiExpectTest {
> +
> + public void testListAvailabilityZones() throws Exception {
> + URI endpoint = URI.create("http://172.16.0.1:8776/v1/50cdb4c60374463198695d9f798fa34d/os-availability-zone");
> + AvailabilityZoneApi api = requestsSendResponses(
> + keystoneAuthWithUsernameAndPasswordAndTenantName,
> + responseWithKeystoneAccess,
> + authenticatedGET().endpoint(endpoint).build(),
> + HttpResponse.builder().statusCode(200).payload(payloadFromResource("/availability_zones_list.json")).build()
> + ).getAvailabilityZoneApi("RegionOne");
> +
> + AvailabilityZone availabilityZoneActual = Iterables.getFirst(api.list(), null);
> + AvailabilityZone availabilityZoneExpected = getTestAvailabilityZone();
> +
> + assertNotNull(availabilityZoneActual);
> + assertTrue(availabilityZoneActual.equals(availabilityZoneExpected));
Better use `assertEquals` here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18570977
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
Comments are mostly minors. Thanks for the PR @inbarsto!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58323984
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
Reopened #560.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#event-182821789
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
Unfotunatly I wont be neae a computer in the comming 24 hours (jewish hollidays are crap...)
If this cant go as is then I will create a new PR after the holliday for the master branch.
Regardless if you can provide an example of how should the change look wirhout the abstract builder i dont seem to find one. I looked at most of rhe nova and cinder domain objects and they all look like this...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58375265
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1754](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1754/) 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/560#issuecomment-58342250
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Adrian Cole <no...@github.com>.
> + * 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.
> + */
> +
> +package org.jclouds.openstack.cinder.v1.domain.zonescoped;
> +
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * zone state for availability zones
> + */
> +public class ZoneState {
Especially for this class, please remove the abstract builder stuff. it makes it very unreadable.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18587781
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by cloudband <no...@github.com>.
> +
> + AvailabilityZone availabilityZoneActual = Iterables.getFirst(api.list(), null);
> + AvailabilityZone availabilityZoneExpected = getTestAvailabilityZone();
> +
> + assertNotNull(availabilityZoneActual);
> + assertEquals(availabilityZoneActual.getName(), availabilityZoneExpected.getName());
> + assertEquals(availabilityZoneActual.getState().available(), availabilityZoneExpected.getState().available());
> + }
> +
> + protected AvailabilityZone getTestAvailabilityZone() {
> + return AvailabilityZone.builder()
> + .name("nova")
> + .isAvailable(ZoneState.builder().available(true).build())
> + .build();
> + }
> +}
done
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18569032
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
Closed #560.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#event-182815889
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
@inbarsto Oh no, you are not driving me nuts at all! I just wanted to clarify since I noticed 2 PRs and I have not had enough coffee yet this morning. :) Thanks for sticking with this!
I do have another Cinder extension related PR that I am submitting this week, so let's get this in and I can update your contribution accordingly. Sound good?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60270488
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1757](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1757/) 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/560#issuecomment-58358567
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
ok i think i am all done let me know if there is anything else..
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58332886
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
yeah i got that a bit late, sorry for the mess Jeremy (pls don't give up on me yet)
I already started to create new PR to master and not to a versioned branch this was i think one of the first PR's,
the intent is to merge to master as well as 1.8 if possible...
sorry for driving u nuts
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60269406
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1262](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1262/) 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/560#issuecomment-58338171
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #233](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/233/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60270766
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1322](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1322/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60272261
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by cloudband <no...@github.com>.
fixed code style
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58341595
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +package org.jclouds.openstack.cinder.v1.extensions;
> +
> +/**
> + * Extension namespaces
> + */
> +public final class ExtensionNamespaces {
> +
> + /**
> + * Admin Action extension
> + */
> + public static final String ADMIN_ACTIONS = "http://docs.openstack.org/ext/admin-actions/api/v1.1";
> +
This class needs a private constructor:
```java
private ExtensionNamespaces() {
throw new AssertionError("intentionally unimplemented");
}
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18592190
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> + private final String name;
> + private final ZoneState state;
> +
> + @ConstructorProperties({
> + "zoneName", "zoneState"
> + })
> + protected AvailabilityZone(String name, ZoneState zoneState) {
> + this.name = checkNotNull(name);
> + this.state = checkNotNull(zoneState);
> + }
> +
> + public String getName() {
> + return this.name;
> + }
> +
> + public ZoneState getState() {
```java
public ZoneState getZoneState() {
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18598293
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
ok, i think i am done :)
pls let me know if there is anything else,
regarding abstract builder (painful subject)
I fixed it here but there is the rest of all domain for nova and cinder (at least maybe more) to fix...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58546988
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #176](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/176/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58345793
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
will do
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60193235
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
Most comments seems valid but note that most of the code is the same as in nova availability zone.
That said lets decide if these comments are valid for nova az as well i'll be happy to do that as well as long as they have the same convention.
It will look strange if method names will be different or code structure will be different between the two lets make this uniform....
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58398774
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Andrew Phillips <no...@github.com>.
Closed #560.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#event-183125467
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
> +
> + private static class ConcreteBuilder extends Builder<ConcreteBuilder> {
> + @Override
> + protected ConcreteBuilder self() {
> + return this;
> + }
> + }
> +
> + private final Boolean available;
> +
> + @ConstructorProperties({"available"})
> + protected ZoneState(Boolean available) {
> + this.available = available;
> + }
> +
> + public boolean available() {
serialization/deserialization works fine (double checked with the live test to be sure)
also note that same names and params appear in nova availability zone...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18748596
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1269](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1269/) 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/560#issuecomment-58553777
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> +
> +import javax.ws.rs.Consumes;
> +import javax.ws.rs.GET;
> +import javax.ws.rs.Path;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * api for cinder availability zones
> + */
> +
> +@Beta
> +@Extension(of = ServiceType.BLOCK_STORAGE, namespace = ExtensionNamespaces.ADMIN_ACTIONS)
> +@RequestFilters(AuthenticateRequest.class)
> +public interface AvailabilityZoneApi {
> +
> +
Remove line
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18597275
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + public void testListAvailabilityZones() throws Exception {
> + URI endpoint = URI.create("http://172.16.0.1:8776/v1/50cdb4c60374463198695d9f798fa34d/os-availability-zone");
> + AvailabilityZoneApi api = requestsSendResponses(
> + keystoneAuthWithUsernameAndPasswordAndTenantName,
> + responseWithKeystoneAccess,
> + authenticatedGET().endpoint(endpoint).build(),
> + HttpResponse.builder().statusCode(200).payload(payloadFromResource("/availability_zones_list.json")).build()
> + ).getAvailabilityZoneApi("RegionOne");
> +
> + AvailabilityZone availabilityZoneActual = Iterables.getFirst(api.list(), null);
> + AvailabilityZone availabilityZoneExpected = getTestAvailabilityZone();
> +
> + assertNotNull(availabilityZoneActual);
> + assertEquals(availabilityZoneActual.getName(), availabilityZoneExpected.getName());
> + assertEquals(availabilityZoneActual.getState().available(), availabilityZoneExpected.getState().available());
The AvailabilityZone already implements equals. Use it instead of asserting the two fields?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18568253
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
> @@ -0,0 +1,8 @@
> +{
> + "availabilityZoneInfo": [{
> + "zoneState": {
> + "available": true
> + },
> + "zoneName": "nova"
> + }]
> +}
done
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18660648
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> + public AvailabilityZoneApiLiveTest() {
> + super();
> + provider = "openstack-cinder";
> + }
> +
> + @BeforeClass(groups = {"integration", "live"})
> + public void setupContext() {
> + super.setup();
> + String zone = Iterables.getFirst(api.getConfiguredZones(), "regionOne");
> + availabilityZoneApi = api.getAvailabilityZoneApi(zone);
> + }
> +
> + public void testListAvailabilityZones() {
> + ImmutableList<? extends AvailabilityZone> cinderZones = availabilityZoneApi.list().toList();
> +
> + assertTrue(cinderZones.size() > 0);
Prefer `isEmpty()` rather than `size() > 0`. This would read `assertTrue(!cinderZones.isEmpty())`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18592410
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Andrew Phillips <no...@github.com>.
Committed to [1.8.x](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;a=commit;h=dcfda5217b39d897e6e8ff975233318ccfbf6cdf)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60348037
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + AvailabilityZone availabilityZoneActual = Iterables.getFirst(api.list(), null);
> + AvailabilityZone availabilityZoneExpected = getTestAvailabilityZone();
> +
> + assertNotNull(availabilityZoneActual);
> + assertEquals(availabilityZoneActual.getName(), availabilityZoneExpected.getName());
> + assertEquals(availabilityZoneActual.getState().available(), availabilityZoneExpected.getState().available());
> + }
> +
> + protected AvailabilityZone getTestAvailabilityZone() {
> + return AvailabilityZone.builder()
> + .name("nova")
> + .isAvailable(ZoneState.builder().available(true).build())
> + .build();
> + }
> +}
The API defines a special behavior for 404 responses (the fallback). Add a test that verifies that the empty iterable is properly returned on a 404.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18568316
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
looks like there are unrelated tests that fail...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58547210
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -79,4 +80,10 @@ SnapshotApi getSnapshotApiForZone(
> QuotaApi getQuotaApi(
> @EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
>
> + /**
> + * provides synchronous access to availability zone features
Remove `synchronous`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18589592
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
@inbarsto Is this the same as the #581 on the 1.8.x branch? Usually, we commit to master first, _then_ do the backport. If its the same, we can just reopen it. Thx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60268579
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * availability zone for cinder
> + */
> +public class AvailabilityZone {
> +
> + public static Builder<?> builder() {
> + return new ConcreteBuilder();
> + }
> +
> + public Builder<?> toBuilder() {
> + return new ConcreteBuilder().fromAvailabilityZone(this);
> + }
> +
> + public abstract static class Builder<T extends Builder<T>> {
+1 Keep them simple cc/ @nacx
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18590262
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
Regarding the builder thing, I meant to leave the following. Just a normal class.
Usage of this abstract/concrete builders where is not needed has been discussed recently, and agree that we could leave it as-is now, just for consistency with the rest of the model.
@jdaggett @everett-toews WDYT about getting rid of all the unnecessary `ConcreteBuilder` stuff? Is there a JIRA for this?
```java
public static Builder builder() {
return new Builder();
}
public Builder toBuilder() {
return new Builder().fromAvailabilityZone(this);
}
public static class Builder {
...
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58330845
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by cloudband <no...@github.com>.
> +
> +/**
> + * availability zone for cinder
> + */
> +public class AvailabilityZone {
> +
> + public static Builder<?> builder() {
> + return new ConcreteBuilder();
> + }
> +
> + public Builder<?> toBuilder() {
> + return new ConcreteBuilder().fromAvailabilityZone(this);
> + }
> +
> + public static abstract class Builder<T extends Builder<T>> {
> + protected abstract T self();
can you point me to an example please?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18568706
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +package org.jclouds.openstack.cinder.v1.extensions;
> +
> +/**
> + * Extension namespaces
> + */
> +public final class ExtensionNamespaces {
> +
> + /**
> + * Admin Action extension
> + */
> + public static final String ADMIN_ACTIONS = "http://docs.openstack.org/ext/admin-actions/api/v1.1";
> +
I dont know if to laugh or to cry
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18619924
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1834](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1834/) 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/560#issuecomment-60276237
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +/**
> + * availability zone for cinder
> + */
> +public class AvailabilityZone {
> +
> + public static Builder<?> builder() {
> + return new ConcreteBuilder();
> + }
> +
> + public Builder<?> toBuilder() {
> + return new ConcreteBuilder().fromAvailabilityZone(this);
> + }
> +
> + public static abstract class Builder<T extends Builder<T>> {
> + protected abstract T self();
I'd remove the ConcreteBuilder and make this one not abstract. Just remove the unnecessary complexity.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18568445
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #172](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/172/) 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/560#issuecomment-58333406
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
> + @Override
> + protected ConcreteBuilder self() {
> + return this;
> + }
> + }
> +
> + private final String name;
> + private final ZoneState state;
> +
> + @ConstructorProperties({
> + "zoneName", "zoneState"
> + })
> + protected AvailabilityZone(String name, ZoneState zoneState) {
> + this.name = checkNotNull(name);
> + this.state = checkNotNull(zoneState);
> + }
Nope. See the json response of the expect test.
It was copied from the jclouds-wire of the live test...
Cinder az is a bit behind nova az....
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18599439
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> + return this
> + .name(in.getName()).isAvailable(in.getState());
> + }
> + }
> +
> + private static class ConcreteBuilder extends Builder<ConcreteBuilder> {
> + @Override
> + protected ConcreteBuilder self() {
> + return this;
> + }
> + }
> +
> + private final String name;
> + private final ZoneState state;
> +
> + @ConstructorProperties({
[minor]
```java
@ConstructorProperties({"zoneName", "zoneState"})
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18597074
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
it was hmm... an experience :stuck_out_tongue_winking_eye:
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58809845
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
The build is unstable due to some checkstyle violations introduced with this change. You can see them in the Jenkins build report or by running `mvn checkstyle:checkstyle` in your project and looking at the generated `checkstyle-results.xml` file. Could you fix them?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58324423
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Adrian Cole <no...@github.com>.
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * availability zone for cinder
> + */
> +public class AvailabilityZone {
> +
> + public static Builder<?> builder() {
> + return new ConcreteBuilder();
> + }
> +
> + public Builder<?> toBuilder() {
> + return new ConcreteBuilder().fromAvailabilityZone(this);
> + }
> +
> + public abstract static class Builder<T extends Builder<T>> {
I also think this shouldn't be an abstract builder. We should stop proliferating these.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18587538
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Ignasi Barrera <no...@github.com>.
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * zone state for availability zones
> + */
> +public class ZoneState {
> +
> + public static Builder<?> builder() {
> + return new ConcreteBuilder();
> + }
> +
> + public Builder<?> toBuilder() {
> + return new ConcreteBuilder().fromZoneState(this);
> + }
> +
> + public static abstract class Builder<T extends Builder<T>> {
Same comment about builders and constructor parameters.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18568142
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +package org.jclouds.openstack.cinder.v1.extensions;
> +
> +/**
> + * Extension namespaces
> + */
> +public final class ExtensionNamespaces {
> +
> + /**
> + * Admin Action extension
> + */
> + public static final String ADMIN_ACTIONS = "http://docs.openstack.org/ext/admin-actions/api/v1.1";
> +
90% of this code is copy paste of existing code from the same branch not old code either ...
I have no problem fixing this but doesnt seem like there is cosistency in the code if i can copy paste some thing and get rejected over 100 lines.... :/
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18620246
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Adrian Cole <no...@github.com>.
> + public abstract static class Builder<T extends Builder<T>> {
> + protected abstract T self();
> +
> + protected ZoneState state;
> + protected String name;
> +
> + /**
> + * @see AvailabilityZone#getName()
> + */
> + public T name(String name) {
> + this.name = name;
> + return self();
> + }
> +
> + /**
> + * @see org.jclouds.openstack.cinder.v1.domain.zonescoped.AvailabilityZone#getState()
nit don't need to fully qualify the path the enclosing class, right?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18587632
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
see PR https://github.com/jclouds/jclouds/pull/580
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60196386
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> +
> + public abstract static class Builder<T extends Builder<T>> {
> + protected abstract T self();
> +
> + protected boolean available;
> +
> +
> + /**
> + * @see ZoneState#available() ()
> + */
> + public T available(Boolean available) {
> + this.available = available;
> + return self();
> + }
> +
> +
Remove line
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18597220
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> + @Override
> + protected ConcreteBuilder self() {
> + return this;
> + }
> + }
> +
> + private final String name;
> + private final ZoneState state;
> +
> + @ConstructorProperties({
> + "zoneName", "zoneState"
> + })
> + protected AvailabilityZone(String name, ZoneState zoneState) {
> + this.name = checkNotNull(name);
> + this.state = checkNotNull(zoneState);
> + }
>From the sources, it looks like there should also be a [`hosts`](https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/contrib/availability_zone.py#L85) field here as well. ???
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18598287
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> +import org.jclouds.Fallbacks;
> +import org.jclouds.openstack.cinder.v1.domain.zonescoped.AvailabilityZone;
> +import org.jclouds.openstack.keystone.v2_0.filters.AuthenticateRequest;
> +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;
> +
> +/**
> + * api for cinder availability zones
Standardization. Please document the intent of the API:
`Provides access to the OpenStack Block Storage (Cinder) Availability Zone Extension API.`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18591896
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> +
> + public ZoneState getState() {
> + return this.state;
> + }
> +
> + @Override
> + public int hashCode() {
> + return Objects.hashCode(name, state);
> + }
> +
> + @Override
> + public boolean equals(Object obj) {
> + if (this == obj) return true;
> + if (obj == null || getClass() != obj.getClass()) return false;
> + AvailabilityZone that = AvailabilityZone.class.cast(obj);
> + return Objects.equal(this.name, that.name) && this.state.equals(that.state);
```java
return Objects.equal(this.name, that.name) && Objects.equal(this.state, that.state);
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18597177
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by cloudband <no...@github.com>.
> + private static class ConcreteBuilder extends Builder<ConcreteBuilder> {
> + @Override
> + protected ConcreteBuilder self() {
> + return this;
> + }
> + }
> +
> + private final String name;
> + private final ZoneState state;
> +
> + @ConstructorProperties({
> + "zoneName", "zoneState"
> + })
> + protected AvailabilityZone(String name, ZoneState zoneState) {
> + this.name = name;
> + this.state = zoneState;
mandatory
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18568152
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Adrian Cole <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +package org.jclouds.openstack.cinder.v1.extensions;
> +
> +/**
> + * Extension namespaces
> + */
> +public final class ExtensionNamespaces {
> +
> + /**
> + * Admin Action extension
> + */
> + public static final String ADMIN_ACTIONS = "http://docs.openstack.org/ext/admin-actions/api/v1.1";
> +
"100, 100 review comments. whah-hah-hah" Count Buildula
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18602815
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> + * api for cinder availability zones
> + */
> +
> +@Beta
> +@Extension(of = ServiceType.BLOCK_STORAGE, namespace = ExtensionNamespaces.ADMIN_ACTIONS)
> +@RequestFilters(AuthenticateRequest.class)
> +public interface AvailabilityZoneApi {
> +
> +
> + /**
> + * List all availability zones
> + *
> + * @return all availability zones
> + */
> + @GET
> + @Path("/os-availability-zone")
For sake of API clarity, please move the `@Path("/os-availability-zone)` and `@Consumes(MediaType.APPLICATION_JSON)` to the interface level:
```java
@Beta
@Extension(of = ServiceType.BLOCK_STORAGE, namespace = ExtensionNamespaces.ADMIN_ACTIONS)
@RequestFilters(AuthenticateRequest.class)
@Consumes(MediaType.APPLICATION_JSON)
@Path("/os-availability-zone")
public interface AvailabilityZoneApi {
...
}
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18591905
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
is this good to go? (at least on master until 1.8 is finished?)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60043919
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Andrew Phillips <no...@github.com>.
>
It's good to go for me, but I'm getting:
```
...\jclouds\jclouds>git am 560.patch.txt
Applying: cinder availability zones api + list call implemented
error: patch failed: apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/CinderApi.java:21
error: apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/CinderApi.java: patch does not apply
Patch failed at 0001 cinder availability zones api + list call implemented
```
:-( Do you think you could open a PR against master for this?
Thanks, @inbarsto!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60192865
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1257](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1257/) 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/560#issuecomment-58315760
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #171](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/171/) 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/560#issuecomment-58330230
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1260](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1260/) 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/560#issuecomment-58332319
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
Unfotunatly I wont be neae a computer in the comming 24 hours (jewish hollidays are crap...)
If this cant go as is then I will create a new PR after the holliday for the master branch.
Regardless if you can provide an example of how should the change look wirhout the abstract builder i dont seem to find one. I looked at most of rhe nova and cinder domain objects and they all look like this...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58373700
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #173](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/173/) 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/560#issuecomment-58336530
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1759](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1759/) 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/560#issuecomment-58374957
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -79,4 +80,10 @@ SnapshotApi getSnapshotApiForZone(
> QuotaApi getQuotaApi(
> @EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
>
> + /**
> + * provides synchronous access to availability zone features
> + */
> + @Delegate
> + AvailabilityZoneApi getAvailabilityZoneApi(
> + @EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
`ZoneToEndpoint
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560/files#r18591130
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
looks like cloudBees finished successfully but after that got an exception.
I dont think it is code related...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-58316271
Re: [jclouds] cinder availability zones api + list call implemented
(#560)
Posted by inbar stolberg <no...@github.com>.
why is this closed?
its not merged to 1.8 which was the intent :(
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/560#issuecomment-60268110