You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by kprobst <no...@github.com> on 2014/11/13 00:59:50 UTC
[jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
You can merge this Pull Request by running:
git pull https://github.com/GoogleCloudPlatform/jclouds-labs-google aggregatedList
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/92
-- Commit Summary --
* AggregatedList for addresses.
* gets addresses aggregated list to work
* adding aggregated list for disks
* disktype
* globaloperations
* update to live test
* fix formatting
* adding test files
* fixes test
-- File Changes --
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/AggregatedListApi.java (180)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/AggregatedListApiLiveTest.java (40)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/AggregatedListApiMockTest.java (80)
A google-compute-engine/src/test/resources/aggregated_address_list.json (131)
A google-compute-engine/src/test/resources/aggregated_address_list_empty.json (115)
A google-compute-engine/src/test/resources/aggregated_disk_list.json (118)
A google-compute-engine/src/test/resources/aggregated_disk_list_empty.json (115)
A google-compute-engine/src/test/resources/aggregated_disktype_list.json (127)
A google-compute-engine/src/test/resources/aggregated_disktype_list_empty.json (115)
A google-compute-engine/src/test/resources/aggregated_global_operation_list.json (98)
A google-compute-engine/src/test/resources/aggregated_global_operation_list_empty.json (115)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/92.patch
https://github.com/jclouds/jclouds-labs-google/pull/92.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
Nice to meet you, Katharina. Thanks for helping out!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63100835
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
> @@ -43,4 +51,36 @@ public void machineTypes() {
>
> assertEquals(machineTypeAsList.size(), 9); // zone count!
> }
> +
> + public void addresses() {
> + Iterator<ListPage<Address>> pageIterator = api().addresses(maxResults(1));
can you please switch to triple space? It is hard to read these files when they are in mixed indent. There's a formatter here, if you like http://wiki.apache.org/jclouds/Coding%20Standards
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92/files#r20270296
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #282](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/282/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-62818526
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #292](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/292/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63195670
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1706](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1706/) 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/92#issuecomment-62819437
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #293](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/293/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63195991
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <ad...@gmail.com>.
Thanks, Kathrin. Sorry about the cheese! So I will have a look tomorrow
morning Pacific time. Unless there's something urgent, I will merge.
Appreciate the hand!
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by kprobst <no...@github.com>.
Ok - I believe I've addressed all the comments. There is a lot of formatting cheese when I used the eclipse formatter, even for existing code.
I had to make user in the operation response optional. I verified that in fact I have an operation where the API does not return a user, so this is legit.
I'll send you another PR renaming the existing list for operations.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63199054
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by kprobst <no...@github.com>.
Ok, thanks for the tip. I'll ping this thread when all comments are addressed and squashed.
As for the PS:, I'm Katharina (or Kathrin for short).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63099334
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1720](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1720/) 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/92#issuecomment-63195783
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
good work! slight style note and a little TODO on the live test, otherwise LGTM
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-62834877
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1719](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1719/) 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/92#issuecomment-63195424
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by kprobst <no...@github.com>.
Nice to meet you as well!
I'll do my best to get this finished up quickly, but likely won't get to it again until later in the day.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63101068
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
> @RequestFilters(OAuthAuthenticationFilter.class)
> @Path("/aggregated")
> @Consumes(APPLICATION_JSON)
> public interface AggregatedListApi {
>
> /**
> - * Retrieves the list of machine type resources available to the specified project.
> - * By default the list as a maximum size of 100, if no options are provided or ListOptions#getMaxResults() has not
> - * been set.
> + * Retrieves the list of machine type resources available to the specified
yeah I think idea doesn't import the 80 char javadoc thing from the eclipse formatter. I'll correct mine.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92/files#r20410405
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
> this.api = api;
> }
>
> - @Override protected Function<String, ListPage<MachineType>> fetchNextPage(final ListOptions options) {
> + @Override
I kindof like same-line but if the formatters are going to argue, I'll change back to this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92/files#r20410415
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1721](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1721/) 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/92#issuecomment-63196108
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
Have a look at OperationApiLiveTest, basically guard with SkipException before you do an assert, if there's nothing there. Many users who run this test won't skip.
https://github.com/GoogleCloudPlatform/jclouds-labs-google/blob/aggregatedList/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/OperationApiLiveTest.java#L82
ps what's your name?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63084263
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
> @@ -64,4 +64,84 @@ public void instancesResponseIs4xx() throws Exception {
>
> assertSent(server, "GET", "/projects/party/aggregated/instances");
> }
> +
nice job on this file!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92/files#r20270356
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
> @@ -43,4 +51,36 @@ public void machineTypes() {
>
> assertEquals(machineTypeAsList.size(), 9); // zone count!
> }
> +
> + public void addresses() {
> + Iterator<ListPage<Address>> pageIterator = api().addresses(maxResults(1));
> + assertFalse(pageIterator.hasNext());
> + }
> +
> + public void disks() {
> + assertOperationDoneSuccessfully(api.disksInZone(DEFAULT_ZONE_NAME).create(DISK_NAME, 1));
> + Iterator<ListPage<Disk>> pageIterator = api().disks(maxResults(1));
> + assertTrue(pageIterator.hasNext());
> + List<Disk> disksAsList = pageIterator.next();
> + assertEquals(disksAsList.size(), 1); // zone count!
> + assertOperationDoneSuccessfully(api.disksInZone(DEFAULT_ZONE_NAME).delete(DISK_NAME));
I'd hate for folks to have to spend money creating disks just to be counted for this test. For resources that are user defined (as opposed to system defined), just loop in the test and throw SkipException if there's nothing to count.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92/files#r20270351
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1722](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1722/) 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/92#issuecomment-63199082
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #283](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/283/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-62819222
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #294](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/294/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63198941
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1714](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1714/) 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-labs-google/pull/92#issuecomment-63099163
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
ping! let's try to keep pull requests <=2-3 days old. That way the flow is better and we don't clobber eachother.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63079075
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by kprobst <no...@github.com>.
> @@ -43,4 +51,36 @@ public void machineTypes() {
>
> assertEquals(machineTypeAsList.size(), 9); // zone count!
> }
> +
> + public void addresses() {
> + Iterator<ListPage<Address>> pageIterator = api().addresses(maxResults(1));
Done. (I used the template and reformatted with it.)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92/files#r20372520
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #288](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/288/) 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-labs-google/pull/92#issuecomment-63099239
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
> @@ -43,4 +51,36 @@ public void machineTypes() {
>
> assertEquals(machineTypeAsList.size(), 9); // zone count!
> }
> +
> + public void addresses() {
> + Iterator<ListPage<Address>> pageIterator = api().addresses(maxResults(1));
> + assertFalse(pageIterator.hasNext());
hmm this is likely to fail, right? what if you don't have any addresses?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92/files#r20270316
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
oh yeah, once you are ready, don't forget to squash :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-62834974
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #291](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/291/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63195270
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1705](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1705/) 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/92#issuecomment-62818913
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
I reworded your commit to "Extends coverage of AggregatedList API: address, disk, disk type, global operations."
Thanks again! in master now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63223815
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
> + @Path("/operations")
> + ListPage<Operation> pageOfGlobalOperations(@Nullable @QueryParam("pageToken") String pageToken, ListOptions listOptions);
> +
> + /** @see #pageOfGlobalOperations(String, ListOptions) */
> + @Named("GlobalOperations:aggregatedList")
> + @GET
> + @Path("/operations")
> + @Transform(OperationPages.class)
> + Iterator<ListPage<Operation>> globalOperations();
> +
> + /** @see #pageOfGlobalOperations(String, ListOptions) */
> + @Named("GlobalOperations:aggregatedList")
> + @GET
> + @Path("/operations")
> + @Transform(OperationPages.class)
> + Iterator<ListPage<Operation>> globalOperations(ListOptions options);
note: we should go back into the operation api and rename the normal list to listGlobal or the like to be consistent here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92/files#r20270264
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by kprobst <no...@github.com>.
Hi Adrian, thanks for the quick reply. I will do this today. I do have one question about your comment on the live test. If I skip over all user-defined ones, what do I assert? I can think of two ways to do this: either I get rid of the test, or I simply make sure that the API call succeeds without an error, but don't pay attention to how many disks are fetched. I'm good with either, but just want to make sure I address your comment as it was intended.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#issuecomment-63083035
Re: [jclouds-labs-google] Extends coverage of AggregatedList API:
address, disk, disk type, global operations (#92)
Posted by Adrian Cole <no...@github.com>.
Closed #92.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/92#event-193906487