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 08:05:46 UTC

[jclouds] list quotas for cinder + expected and live tests (#178)

list quotas for Openstack cinder + live and expect tests
You can merge this Pull Request by running:

  git pull https://github.com/inbarsto/jclouds-1 quotas2

Or you can view, comment on it, or merge it online at:

  https://github.com/jclouds/jclouds/pull/178

-- Commit Summary --

  * list quotas for cinder + expected and live tests

-- File Changes --

    M apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/CinderApi.java (22)
    A apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/domain/VolumeQuota.java (176)
    A apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/features/QuotaApi.java (56)
    A apis/openstack-cinder/src/test/java/org/jclouds/openstack/cinder/v1/features/QuotasApiExpectedTest.java (55)
    A apis/openstack-cinder/src/test/java/org/jclouds/openstack/cinder/v1/features/QuotasApiLiveTest.java (60)
    A apis/openstack-cinder/src/test/resources/quotas.json (7)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/178.patch
https://github.com/jclouds/jclouds/pull/178.diff

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
>     /**
>      * Provides synchronous access to Snapshot features.
>      */
>     @Delegate
>     SnapshotApi getSnapshotApiForZone(
>           @EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
> +   /**
> +    * provides synchronous access to quotas features.

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962911

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
>  
>  /**
>   * Provides synchronous access to Cinder.
> - *  
> + *
>   * @see CinderAsyncApi

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962909

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> +package org.jclouds.openstack.cinder.v1.features;
> +
> +import org.jclouds.http.HttpResponse;
> +import org.jclouds.openstack.cinder.v1.domain.VolumeQuota;
> +import org.jclouds.openstack.cinder.v1.internal.BaseCinderApiExpectTest;
> +import org.testng.annotations.Test;
> +
> +import java.net.URI;
> +
> +import static org.testng.Assert.assertEquals;
> +
> +/**
> + * @author inbar stolberg
> + */
> +@Test(groups = "unit", testName = "QuotaApiExpectedTest")
> +public class QuotasApiExpectedTest extends BaseCinderApiExpectTest {

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962921

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #314](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/314/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26394066

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #773](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/773/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26312119

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #762](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/762/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26237733

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
>   * @see CinderAsyncApi
>   * @see <a href="http://api.openstack.org/">API Doc</a>
> - * @author Everett Toews

Go ahead and add yourself as an author if you like but don't remove other people.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6957235

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> +      provider = "openstack-cinder";
> +   }
> +
> +   @BeforeClass(groups = {"integration", "live"})
> +   public void setupContext() {
> +      super.setup();
> +      String zone = api.getConfiguredZones().iterator().next();
> +      quotaApi = api.getQuotaApi(zone);
> +   }
> +
> +   public void testGetStorageQuotas() throws ExecutionException, InterruptedException {
> +      VolumeQuota volumeQuota = quotaApi.getByTenant("non");
> +
> +      assertNotNull(volumeQuota.getGigabytes());
> +      assertNotNull(volumeQuota.getVolumes());
> +      assertNotNull(volumeQuota.getSnapshots());

All of these values are ints. assertNotNull isn't the best assertion. assertTrue(value >= 0) would be better.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6958360

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Andrew Phillips <no...@github.com>.
Thanks @inbarsto and @everett-toews! Just one small comment for next time: a commit message that has _something_ descriptive on the first line (besides just the JIRA issue) makes for nicer reading. So e.g.
```
JCLOUDS-348: list quotas for cinder + expected and live tests
```
rather than
```
JCLOUDS-348
list quotas for cinder + expected and live tests
```
See e.g. https://git-wip-us.apache.org/repos/asf?p=incubator-jclouds.git;a=summary for an example ;-)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26751226

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
jira ticket is https://issues.apache.org/jira/browse/JCLOUDS-348 (i hope i got this right please let me know if i need to add/change anything )

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26393215

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> + * @author Inbar Stolberg
> + * @see QuotaApi
> + * @see <a href="http://api.openstack.org/">API Doc</a>
> + */
> +@SkipEncoding({'/', '='})
> +@RequestFilters(AuthenticateRequest.class)
> +@Path("/os-quota-sets")
> +//@org.jclouds.rest.annotations.Endpoint(BlockStorage.class)
> +public interface QuotaApi {
> +
> +
> +   @GET
> +   @SelectJson("quota_set")
> +   @Consumes(MediaType.APPLICATION_JSON)
> +   @Path("/{tenant_id}")
> +   @Fallback(Fallbacks.EmptyFluentIterableOnNotFoundOr404.class)

Wrong Fallback. Should be NullOnNotFoundOr404.class.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6957496

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #300](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/300/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26237719

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> @@ -0,0 +1,7 @@
> +{"quota_set": {
> +    "gigabytes": 1000,
> +    "volumes": 10,
> +    "snapshots": 20,
> +    "id": "demo",
> +    "security_groups": 10

What is "security_groups" doing in here?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6958375

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #770](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/770/) 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/178#issuecomment-26310385

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> +import javax.ws.rs.GET;
> +import javax.ws.rs.Path;
> +import javax.ws.rs.PathParam;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * Provides asynchronous access to Quota via their REST API.
> + *
> + * @author Inbar Stolberg
> + * @see QuotaApi
> + * @see <a href="http://api.openstack.org/">API Doc</a>
> + */
> +@SkipEncoding({'/', '='})
> +@RequestFilters(AuthenticateRequest.class)
> +@Path("/os-quota-sets")
> +//@org.jclouds.rest.annotations.Endpoint(BlockStorage.class)

Remove commented out code.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6957415

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> +   private QuotaApi quotaApi;
> +
> +   public QuotasApiLiveTest() {
> +      super();
> +      provider = "openstack-cinder";
> +   }
> +
> +   @BeforeClass(groups = {"integration", "live"})
> +   public void setupContext() {
> +      super.setup();
> +      String zone = api.getConfiguredZones().iterator().next();
> +      quotaApi = api.getQuotaApi(zone);
> +   }
> +
> +   public void testGetStorageQuotas() throws ExecutionException, InterruptedException {
> +      VolumeQuota volumeQuota = quotaApi.getByTenant("non");

Why did you use "non" here?

Why not use one of the default DevStack users like "demo"?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6958319

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <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.cinder.v1.features;
> +
> +import com.google.common.collect.Iterables;
> +import org.jclouds.openstack.cinder.v1.domain.VolumeQuota;
> +import org.jclouds.openstack.cinder.v1.internal.BaseCinderApiLiveTest;
> +import org.testng.annotations.BeforeClass;
> +import org.testng.annotations.Test;
> +
> +import java.util.concurrent.ExecutionException;
> +
> +import static org.testng.Assert.assertNotNull;

Unused import.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r7014330

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #801](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/801/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26671776

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #338](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/338/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26671773

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> + * @author inbar stolberg
> + */
> +@Test(groups = "live", testName = "QuotasApiLiveTest", singleThreaded = true)
> +public class QuotasApiLiveTest extends BaseCinderApiLiveTest {
> +
> +   private QuotaApi quotaApi;
> +
> +   public QuotasApiLiveTest() {
> +      super();
> +      provider = "openstack-cinder";
> +   }
> +
> +   @BeforeClass(groups = {"integration", "live"})
> +   public void setupContext() {
> +      super.setup();
> +      String zone = api.getConfiguredZones().iterator().next();

Use `Iterables.getFirst()` instead

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6958305

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> +      provider = "openstack-cinder";
> +   }
> +
> +   @BeforeClass(groups = {"integration", "live"})
> +   public void setupContext() {
> +      super.setup();
> +      String zone = api.getConfiguredZones().iterator().next();
> +      quotaApi = api.getQuotaApi(zone);
> +   }
> +
> +   public void testGetStorageQuotas() throws ExecutionException, InterruptedException {
> +      VolumeQuota volumeQuota = quotaApi.getByTenant("non");
> +
> +      assertNotNull(volumeQuota.getGigabytes());
> +      assertNotNull(volumeQuota.getVolumes());
> +      assertNotNull(volumeQuota.getSnapshots());

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962945

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
>     /**
>      * Provides synchronous access to Snapshot features.
>      */
>     @Delegate
>     SnapshotApi getSnapshotApiForZone(
>           @EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
> +   /**
> +    * provides synchronous access to quotas features.

Capitalize provides

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6957254

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> +/**
> + * @author Inbar Stolberg
> + *         Date: 8/7/13
> + *         Time: 11:55 AM
> + */
> +public class VolumeQuota {
> +
> +   public static Builder<?> builder() {
> +      return new ConcreteBuilder();
> +   }
> +
> +   public Builder<?> toBuilder() {
> +      return new ConcreteBuilder().fromVolumeQuotas(this);
> +   }
> +
> +   public static abstract class Builder<T extends Builder<T>> {

You should be able to simplify this to `public static class Builder`

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6957381

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> +import javax.ws.rs.GET;
> +import javax.ws.rs.Path;
> +import javax.ws.rs.PathParam;
> +import javax.ws.rs.core.MediaType;
> +
> +/**
> + * Provides asynchronous access to Quota via their REST API.
> + *
> + * @author Inbar Stolberg
> + * @see QuotaApi
> + * @see <a href="http://api.openstack.org/">API Doc</a>
> + */
> +@SkipEncoding({'/', '='})
> +@RequestFilters(AuthenticateRequest.class)
> +@Path("/os-quota-sets")
> +//@org.jclouds.rest.annotations.Endpoint(BlockStorage.class)

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962918

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
Nice work! Just remove that one unused import and squash down the commits (if you need a hand squashing the commits see steps 17-22 of [An Annotated GitHub Workflow](http://blog.phymata.com/2013/01/25/an-annotated-github-workflow/)) so there is only one commit for the PR.

+1

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26453616

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> +   public void setupContext() {
> +      super.setup();
> +      String zone = api.getConfiguredZones().iterator().next();
> +      quotaApi = api.getQuotaApi(zone);
> +   }
> +
> +   public void testGetStorageQuotas() throws ExecutionException, InterruptedException {
> +      VolumeQuota volumeQuota = quotaApi.getByTenant("non");
> +
> +      assertNotNull(volumeQuota.getGigabytes());
> +      assertNotNull(volumeQuota.getVolumes());
> +      assertNotNull(volumeQuota.getSnapshots());
> +
> +   }
> +
> +

Remove these empty lines.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6958366

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #517](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/517/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26310868

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
Fix/Try those last couple of comments, squash down the commits with your next push, and this is good to go. Thanks!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26346457

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> +/**
> + * @author Inbar Stolberg
> + *         Date: 8/7/13
> + *         Time: 11:55 AM
> + */
> +public class VolumeQuota {
> +
> +   public static Builder<?> builder() {
> +      return new ConcreteBuilder();
> +   }
> +
> +   public Builder<?> toBuilder() {
> +      return new ConcreteBuilder().fromVolumeQuotas(this);
> +   }
> +
> +   public static abstract class Builder<T extends Builder<T>> {

if I will won't it deviate from the standard already used?

see classes:

 https://github.com/jclouds/jclouds/blob/master/apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/domain/Snapshot.java

https://github.com/jclouds/jclouds/blob/master/apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/domain/Volume.java

https://github.com/jclouds/jclouds/blob/master/apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/domain/VolumeType.java

https://github.com/jclouds/jclouds/blob/master/apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/domain/VolumeAttachment.java

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962993

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #800](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/800/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26669881

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #337](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/337/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26669889

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> + * 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;
> +
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Inbar Stolberg
> + *         Date: 8/7/13
> + *         Time: 11:55 AM

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962913

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> +      }
> +
> +      /**
> +       * @see VolumeQuota#getSnapshots()
> +       */
> +      public T snapshots(int snapshots) {
> +         this.snapshots = snapshots;
> +         return self();
> +      }
> +
> +
> +      public VolumeQuota build() {
> +         return new VolumeQuota(id, volumes, gigabytes, snapshots);
> +      }
> +
> +      public T fromVolumeQuotas(VolumeQuota in) {

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6992577

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> +
> +import java.beans.ConstructorProperties;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class VolumeQuota {
> +
> +   private final String id;
> +   private final int volumes;
> +   private final int gigabytes;
> +   private final int snapshots;
> +
> +   @ConstructorProperties({

tried it works :)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6992578

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> +package org.jclouds.openstack.cinder.v1.features;
> +
> +import org.jclouds.http.HttpResponse;
> +import org.jclouds.openstack.cinder.v1.domain.VolumeQuota;
> +import org.jclouds.openstack.cinder.v1.internal.BaseCinderApiExpectTest;
> +import org.testng.annotations.Test;
> +
> +import java.net.URI;
> +
> +import static org.testng.Assert.assertEquals;
> +
> +/**
> + * @author inbar stolberg
> + */
> +@Test(groups = "unit", testName = "QuotaApiExpectedTest")
> +public class QuotasApiExpectedTest extends BaseCinderApiExpectTest {

Should be QuotasApiExpectTest (don't forget the testName above too.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6958219

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
When submitting OpenStack code, please make it as easy as possible for the reviewer to run the live tests. That usually means including the code of a localrc file for DevStack for anything beyond the most basic DevStack deploy. It also means including the commands to run the live tests. In this case I used

    mvn clean test -Plive -Dtest.openstack-cinder.endpoint=http://166.78.236.99:5000/v2.0/ -Dtest.openstack-cinder.identity=admin:admin -Dtest.openstack-cinder.credential=devstack -Dtest=QuotasApiLiveTest

I should be able to get to the rest of your PRs tomorrow.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26295689

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> + * @author inbar stolberg
> + */
> +@Test(groups = "live", testName = "QuotasApiLiveTest", singleThreaded = true)
> +public class QuotasApiLiveTest extends BaseCinderApiLiveTest {
> +
> +   private QuotaApi quotaApi;
> +
> +   public QuotasApiLiveTest() {
> +      super();
> +      provider = "openstack-cinder";
> +   }
> +
> +   @BeforeClass(groups = {"integration", "live"})
> +   public void setupContext() {
> +      super.setup();
> +      String zone = api.getConfiguredZones().iterator().next();

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962926

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
>   * @see CinderAsyncApi
>   * @see <a href="http://api.openstack.org/">API Doc</a>
> - * @author Everett Toews

well this is awkward :) (i noticed in #161 that i removed adrian coll by accident as well :/ ) 

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962639

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #308](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/308/) 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/178#issuecomment-26310387

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

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/178#issuecomment-26720249

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
>  
>  /**
>   * Provides synchronous access to Cinder.
> - *  
> + *
>   * @see CinderAsyncApi

Go ahead and delete this @see CinderAsyncApi as it's no longer valid.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6957249

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #523](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/523/) 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/178#issuecomment-26394739

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
Please create a [JIRA](https://issues.apache.org/jira/browse/JCLOUDS) for this issue and then paste the link to it in the original PR description above.

Also let me know your JIRA username and I'll add you as a contributor!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26371247

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> +      }
> +
> +      /**
> +       * @see VolumeQuota#getSnapshots()
> +       */
> +      public T snapshots(int snapshots) {
> +         this.snapshots = snapshots;
> +         return self();
> +      }
> +
> +
> +      public VolumeQuota build() {
> +         return new VolumeQuota(id, volumes, gigabytes, snapshots);
> +      }
> +
> +      public T fromVolumeQuotas(VolumeQuota in) {

This should be fromVolumeQuota (singular).

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6974926

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> + * 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;
> +
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Inbar Stolberg
> + *         Date: 8/7/13
> + *         Time: 11:55 AM

Remove Date and Time lines

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6957263

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> +package org.jclouds.openstack.cinder.v1.domain;
> +
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Inbar Stolberg
> + *         Date: 8/7/13
> + *         Time: 11:55 AM
> + */
> +public class VolumeQuota {
> +
> +   public static Builder<?> builder() {

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962915

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> @@ -0,0 +1,7 @@
> +{"quota_set": {
> +    "gigabytes": 1000,
> +    "volumes": 10,
> +    "snapshots": 20,
> +    "id": "demo",
> +    "security_groups": 10

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962948

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #776](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/776/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26394070

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> +   public void setupContext() {
> +      super.setup();
> +      String zone = api.getConfiguredZones().iterator().next();
> +      quotaApi = api.getQuotaApi(zone);
> +   }
> +
> +   public void testGetStorageQuotas() throws ExecutionException, InterruptedException {
> +      VolumeQuota volumeQuota = quotaApi.getByTenant("non");
> +
> +      assertNotNull(volumeQuota.getGigabytes());
> +      assertNotNull(volumeQuota.getVolumes());
> +      assertNotNull(volumeQuota.getSnapshots());
> +
> +   }
> +
> +

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962947

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> +   private QuotaApi quotaApi;
> +
> +   public QuotasApiLiveTest() {
> +      super();
> +      provider = "openstack-cinder";
> +   }
> +
> +   @BeforeClass(groups = {"integration", "live"})
> +   public void setupContext() {
> +      super.setup();
> +      String zone = api.getConfiguredZones().iterator().next();
> +      quotaApi = api.getQuotaApi(zone);
> +   }
> +
> +   public void testGetStorageQuotas() throws ExecutionException, InterruptedException {
> +      VolumeQuota volumeQuota = quotaApi.getByTenant("non");

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962943

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> +/**
> + * @author Inbar Stolberg
> + *         Date: 8/7/13
> + *         Time: 11:55 AM
> + */
> +public class VolumeQuota {
> +
> +   public static Builder<?> builder() {
> +      return new ConcreteBuilder();
> +   }
> +
> +   public Builder<?> toBuilder() {
> +      return new ConcreteBuilder().fromVolumeQuotas(this);
> +   }
> +
> +   public static abstract class Builder<T extends Builder<T>> {

I see what you mean. Within this package we should keep things consistent. Leave it as is.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6974854

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #311](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/311/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178#issuecomment-26312101

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> +package org.jclouds.openstack.cinder.v1.domain;
> +
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Inbar Stolberg
> + *         Date: 8/7/13
> + *         Time: 11:55 AM
> + */
> +public class VolumeQuota {
> +
> +   public static Builder<?> builder() {

Move all Builder code to the bottom of the file. See https://github.com/jclouds/jclouds-labs-openstack/blob/master/openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/AllocationPool.java for an example.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6957306

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by inbar stolberg <no...@github.com>.
> + * @author Inbar Stolberg
> + * @see QuotaApi
> + * @see <a href="http://api.openstack.org/">API Doc</a>
> + */
> +@SkipEncoding({'/', '='})
> +@RequestFilters(AuthenticateRequest.class)
> +@Path("/os-quota-sets")
> +//@org.jclouds.rest.annotations.Endpoint(BlockStorage.class)
> +public interface QuotaApi {
> +
> +
> +   @GET
> +   @SelectJson("quota_set")
> +   @Consumes(MediaType.APPLICATION_JSON)
> +   @Path("/{tenant_id}")
> +   @Fallback(Fallbacks.EmptyFluentIterableOnNotFoundOr404.class)

fixed

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6962919

Re: [jclouds] list quotas for cinder + expected and live tests (#178)

Posted by Everett Toews <no...@github.com>.
> +
> +import java.beans.ConstructorProperties;
> +
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Inbar Stolberg
> + */
> +public class VolumeQuota {
> +
> +   private final String id;
> +   private final int volumes;
> +   private final int gigabytes;
> +   private final int snapshots;
> +
> +   @ConstructorProperties({

When you're class fields match the names in the JSON perfectly, I don't think you need @ConstructorProperties. Try removing it and see if everything still works.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/178/files#r6974909