You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by danbroudy <no...@github.com> on 2014/10/09 00:53:48 UTC
[jclouds-labs-google] Add coverage of DiskType Api (#56)
I figured before adding support for SSDs it makes sense to add coverage of the DiskTypes Api.
DiskTypes Api Documentation: https://cloud.google.com/compute/docs/reference/latest/diskTypes
The implementation is very similar to MachineTypes Api. Refactoring redundancies may be desirable in the future.
Also like MachineTypes this only covers the "get" and "list" operations and does not yet cover the "aggregatedList" operation.
You can merge this Pull Request by running:
git pull https://github.com/GoogleCloudPlatform/jclouds-labs-google ssd
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/56
-- Commit Summary --
* Added support for DiskTypes Api. Missing Testing
* Merge branch 'master' of https://github.com/GoogleCloudPlatform/jclouds-labs-google into ssd
* Adding missing files
* Added testing for DiskType Api coverage
* fixing indentation and replacing tabs with spaces
* added newlines at end of some files
-- File Changes --
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineApi.java (10)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/DiskType.java (165)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Resource.java (2)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/DiskTypeApi.java (142)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseDiskTypes.java (64)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/DiskTypeApiExpectTest.java (113)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/DiskTypeApiLiveTest.java (73)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseDiskTypeListTest.java (64)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseDiskTypeTest.java (50)
A google-compute-engine/src/test/resources/disktype.json (11)
A google-compute-engine/src/test/resources/disktype_list.json (27)
A google-compute-engine/src/test/resources/disktype_list_central1b.json (17)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/56.patch
https://github.com/jclouds/jclouds-labs-google/pull/56.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by danbroudy <no...@github.com>.
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "validDiskSize",
> + "deprecated", "zone", "defaultDiskSizeGb"
> + })
> + private DiskType(String id, Date creationTimestamp, URI selfLink, String name, String description,
> + String validDiskSize, Deprecated deprecated, String zone, long defaultDiskSizeGb){
> + super(Kind.DISK_TYPE, id == null ? "" : id, creationTimestamp, selfLink, name, description);
> + this.validDiskSize = checkNotNull(validDiskSize, "validDiskSize of %s", name);
> + this.deprecated = fromNullable(deprecated);
> + this.zone = checkNotNull(zone, "zone of %s", name);
> + this.defaultDiskSizeGb = defaultDiskSizeGb;
> + }
> +
> + /**
> + * @return An optional textual description of the valid disk size. For example, "10GB-10TB."
I changed DiskType.ValidDiskSize to Optional in the rebased and squashed commit below.
I realize now it may have been better to not squash the commit so the minor changes would have been more noticeable.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r19097600
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by danbroudy <no...@github.com>.
> + * @see DiskTypeApi#listAtMarkerInZone(String, String, org.jclouds.googlecomputeengine.options.ListOptions)
> + *
> + * @param zone the zone to list in
> + * @return a Paged, Fluent Iterable that is able to fetch additional pages when required
> + * @see PagedIterable
> + */
> + @Named("DiskTypes:list")
> + @GET
> + @Path("/zones/{zone}/diskTypes")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseDiskTypes.class)
> + @Transform(ParseDiskTypes.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<DiskType> listInZone(@PathParam("zone") String zone, ListOptions listOptions);
> +
> +}
Ok thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r19058430
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by Ignasi Barrera <no...@github.com>.
> + * @see DiskTypeApi#listAtMarkerInZone(String, String, org.jclouds.googlecomputeengine.options.ListOptions)
> + *
> + * @param zone the zone to list in
> + * @return a Paged, Fluent Iterable that is able to fetch additional pages when required
> + * @see PagedIterable
> + */
> + @Named("DiskTypes:list")
> + @GET
> + @Path("/zones/{zone}/diskTypes")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseDiskTypes.class)
> + @Transform(ParseDiskTypes.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<DiskType> listInZone(@PathParam("zone") String zone, ListOptions listOptions);
> +
> +}
OK, I've started refactoring the pagination in all apis, so let's merge this PR first and I'll take care of changing the pagination methods afterwards :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r19056471
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1467](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1467/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56#issuecomment-58924025
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by danbroudy <no...@github.com>.
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "validDiskSize",
> + "deprecated", "zone", "defaultDiskSizeGb"
> + })
> + private DiskType(String id, Date creationTimestamp, URI selfLink, String name, String description,
> + String validDiskSize, Deprecated deprecated, String zone, long defaultDiskSizeGb){
> + super(Kind.DISK_TYPE, id == null ? "" : id, creationTimestamp, selfLink, name, description);
> + this.validDiskSize = checkNotNull(validDiskSize, "validDiskSize of %s", name);
> + this.deprecated = fromNullable(deprecated);
> + this.zone = checkNotNull(zone, "zone of %s", name);
> + this.defaultDiskSizeGb = defaultDiskSizeGb;
> + }
> +
> + /**
> + * @return An optional textual description of the valid disk size. For example, "10GB-10TB."
Sounds good!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r19096171
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by Ignasi Barrera <no...@github.com>.
> + * @see DiskTypeApi#listAtMarkerInZone(String, String, org.jclouds.googlecomputeengine.options.ListOptions)
> + *
> + * @param zone the zone to list in
> + * @return a Paged, Fluent Iterable that is able to fetch additional pages when required
> + * @see PagedIterable
> + */
> + @Named("DiskTypes:list")
> + @GET
> + @Path("/zones/{zone}/diskTypes")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseDiskTypes.class)
> + @Transform(ParseDiskTypes.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<DiskType> listInZone(@PathParam("zone") String zone, ListOptions listOptions);
> +
> +}
jclouds APIs with pagination usually provide two methods to implement the *list* calls:
* One method that does not have pagination parameters and returns a `PagedIterable` (an Iterable that knows how to jump to the next page).
* And a method that accepts custom options (such as the page number, marker, whatever), that returns an `IterableWithMarker` (a single page, with the marker to the next one, that can be turn into a `PagedIterable`, if needed).
With this approach, would the last two *list* methods in this class be enough? The previous `listFirstPageInZone` and `listAtMarkerInZone` seem redundant?
Also, could the last method (the one that accepts the `ListOptions` parameter) be changed to return just an `IterableWithMarker`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r18989334
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "validDiskSize",
> + "deprecated", "zone", "defaultDiskSizeGb"
> + })
> + private DiskType(String id, Date creationTimestamp, URI selfLink, String name, String description,
> + String validDiskSize, Deprecated deprecated, String zone, long defaultDiskSizeGb){
> + super(Kind.DISK_TYPE, id == null ? "" : id, creationTimestamp, selfLink, name, description);
> + this.validDiskSize = checkNotNull(validDiskSize, "validDiskSize of %s", name);
> + this.deprecated = fromNullable(deprecated);
> + this.zone = checkNotNull(zone, "zone of %s", name);
> + this.defaultDiskSizeGb = defaultDiskSizeGb;
> + }
> +
> + /**
> + * @return An optional textual description of the valid disk size. For example, "10GB-10TB."
I'd change it to return an `Optional`, for consistency with the `deprecated` field, and for the implicit benefits of using Optional vs null.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r19083132
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "validDiskSize",
> + "deprecated", "zone", "defaultDiskSizeGb"
> + })
> + private DiskType(String id, Date creationTimestamp, URI selfLink, String name, String description,
> + String validDiskSize, Deprecated deprecated, String zone, long defaultDiskSizeGb){
> + super(Kind.DISK_TYPE, id == null ? "" : id, creationTimestamp, selfLink, name, description);
> + this.validDiskSize = checkNotNull(validDiskSize, "validDiskSize of %s", name);
> + this.deprecated = fromNullable(deprecated);
> + this.zone = checkNotNull(zone, "zone of %s", name);
> + this.defaultDiskSizeGb = defaultDiskSizeGb;
> + }
> +
> + /**
> + * @return An optional textual description of the valid disk size. For example, "10GB-10TB."
NP! Thanks for the quick fixes!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r19112513
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #153](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/153/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56#issuecomment-58441727
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1477](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1477/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56#issuecomment-59425094
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1492](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1492/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56#issuecomment-59797764
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by danbroudy <no...@github.com>.
This commit has more overlap with #58 then I intended. I am going to clean this up and rebase.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56#issuecomment-59402375
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1442](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1442/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56#issuecomment-58441605
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by Ignasi Barrera <no...@github.com>.
> + * @see DiskTypeApi#listAtMarkerInZone(String, String, org.jclouds.googlecomputeengine.options.ListOptions)
> + *
> + * @param zone the zone to list in
> + * @return a Paged, Fluent Iterable that is able to fetch additional pages when required
> + * @see PagedIterable
> + */
> + @Named("DiskTypes:list")
> + @GET
> + @Path("/zones/{zone}/diskTypes")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseDiskTypes.class)
> + @Transform(ParseDiskTypes.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<DiskType> listInZone(@PathParam("zone") String zone, ListOptions listOptions);
> +
> +}
Actually, I've just seen that the "redundant" pagination methods is a pattern in this provider. If you agree that makes sense to only keep the last two, I'll take care of addressing the rest of APIs :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r18989611
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #162](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/162/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56#issuecomment-59424434
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #156](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/156/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56#issuecomment-58923625
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by Ignasi Barrera <no...@github.com>.
> + * @see DiskTypeApi#listAtMarkerInZone(String, String, org.jclouds.googlecomputeengine.options.ListOptions)
> + *
> + * @param zone the zone to list in
> + * @return a Paged, Fluent Iterable that is able to fetch additional pages when required
> + * @see PagedIterable
> + */
> + @Named("DiskTypes:list")
> + @GET
> + @Path("/zones/{zone}/diskTypes")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseDiskTypes.class)
> + @Transform(ParseDiskTypes.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<DiskType> listInZone(@PathParam("zone") String zone, ListOptions listOptions);
> +
> +}
BTW, it should be as easy as adding the pageToken (or marker) parameter in the `ListOptions`, and all the redundant methods would be really redundant and we should be able to remove them.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r18989742
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "validDiskSize",
> + "deprecated", "zone", "defaultDiskSizeGb"
> + })
> + private DiskType(String id, Date creationTimestamp, URI selfLink, String name, String description,
> + String validDiskSize, Deprecated deprecated, String zone, long defaultDiskSizeGb){
> + super(Kind.DISK_TYPE, id == null ? "" : id, creationTimestamp, selfLink, name, description);
> + this.validDiskSize = checkNotNull(validDiskSize, "validDiskSize of %s", name);
> + this.deprecated = fromNullable(deprecated);
> + this.zone = checkNotNull(zone, "zone of %s", name);
> + this.defaultDiskSizeGb = defaultDiskSizeGb;
> + }
> +
> + /**
> + * @return An optional textual description of the valid disk size. For example, "10GB-10TB."
[minor] If this is optional, should the field be nullable? Otherwise remove the "optional" from the javadoc?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r18988832
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #165](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/165/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56#issuecomment-59797259
Re: [jclouds-labs-google] Add coverage of DiskType Api (#56)
Posted by danbroudy <no...@github.com>.
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "validDiskSize",
> + "deprecated", "zone", "defaultDiskSizeGb"
> + })
> + private DiskType(String id, Date creationTimestamp, URI selfLink, String name, String description,
> + String validDiskSize, Deprecated deprecated, String zone, long defaultDiskSizeGb){
> + super(Kind.DISK_TYPE, id == null ? "" : id, creationTimestamp, selfLink, name, description);
> + this.validDiskSize = checkNotNull(validDiskSize, "validDiskSize of %s", name);
> + this.deprecated = fromNullable(deprecated);
> + this.zone = checkNotNull(zone, "zone of %s", name);
> + this.defaultDiskSizeGb = defaultDiskSizeGb;
> + }
> +
> + /**
> + * @return An optional textual description of the valid disk size. For example, "10GB-10TB."
The javadoc is based on the website. Should I change it to an Optional<String> or just add the @Nullable annotation somewhere?
Im relatively new to Java.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/56/files#r19058429