You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Adrian Cole <no...@github.com> on 2014/11/05 00:10:04 UTC
[jclouds-labs-google] Make instance api prettier. (#80)
@nacx @danbroudy lemme know if you agree this is prettier. If so, I will help do the other apis.
You can merge this Pull Request by running:
git pull https://github.com/adriancole/jclouds-labs-google adrian.pretty-instance
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/80
-- Commit Summary --
* Make instance api prettier.
-- File Changes --
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineApi.java (5)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceAdapter.java (97)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/domain/InstanceInZone.java (36)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/domain/MachineTypeInZone.java (36)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/domain/SlashEncodedIds.java (62)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/extensions/GoogleComputeEngineSecurityGroupExtension.java (27)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/InstanceInZoneToNodeMetadata.java (12)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/MachineTypeInZoneToHardware.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/config/GoogleComputeEngineHttpApiModule.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/InstanceApi.java (157)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseInstances.java (5)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/options/ListOptions.java (19)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/functions/InstanceInZoneToNodeMetadataTest.java (4)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiExpectTest.java (78)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiLiveTest.java (42)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/TargetPoolApiLiveTest.java (10)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/80.patch
https://github.com/jclouds/jclouds-labs-google/pull/80.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1632](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1632/) 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/80#issuecomment-61734690
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #226](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/226/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80#issuecomment-61733708
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by danbroudy <no...@github.com>.
> */
> @Delegate
> - @Path("/projects/{project}")
Sounds good.
I'm not familiar with how aggregated lists work. I'll look around to see if the cross-project aggregated image api has been requested.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80/files#r19848093
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by danbroudy <no...@github.com>.
> */
> @Delegate
> - @Path("/projects/{project}")
I find looking at the Google provided Api documentation helpful. Placing the aggregatedList calls in a separate Api could be confusing. Due to that, I lean towards an option that mirrors the documentation's format.
That being said option 3 is a cleaner option.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80/files#r19846154
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #224](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/224/) 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/80#issuecomment-61732601
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by Andrea Turli <no...@github.com>.
> */
> @Delegate
> - @Path("/projects/{project}")
@adriancole I think I lean toward option 3
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80/files#r19845642
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #225](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/225/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80#issuecomment-61733449
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1630](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1630/) 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/80#issuecomment-61732904
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by danbroudy <no...@github.com>.
> */
> @Delegate
> - @Path("/projects/{project}")
One hesitation here about moving '/zones/{zone}' up a level. [AggregatedList](https://cloud.google.com/compute/docs/reference/latest/instances/aggregatedList) is a very useful call in the InstanceApi that does not have a specific zone.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80/files#r19844861
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1633](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1633/) 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/80#issuecomment-61735268
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by Adrian Cole <no...@github.com>.
thx for the feedback folks. merged. I will propagate this design across all the other apis, then add the aggregated stuff.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80#issuecomment-61744821
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by Adrian Cole <no...@github.com>.
> */
> @Delegate
> - @Path("/projects/{project}")
cool call! Hmm so, there are a couple ways to address this.
1. `getInstanceApi(project, null).aggregatedList()` // add to InstanceApi and rejig to allow ignore zone
2. `listInstances()` // add to top-level GoogleComputeEngineApi interface
do either of these sound better, or something else?
If it is only one call that is aggregate, then seems 2 might work...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80/files#r19845147
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by Adrian Cole <no...@github.com>.
live tests are sameish
```
Failed tests:
GoogleComputeEngineServiceLiveTest>BaseComputeServiceLiveTest.testCreateAndRunAService:706->BaseComputeServiceLiveTest.createAndRunAServiceInGroup:723 » UncheckedExecution
InstanceApiLiveTest.testDeleteInstance:221->BaseGoogleComputeEngineApiLiveTest.assertZoneOperationDoneSuccessfully:106 » NullPointer
GoogleComputeEngineServiceLiveTest>BaseComputeServiceLiveTest.testAScriptExecutionAfterBootWithBasicTemplate:223 » RunNodes
GoogleComputeEngineServiceLiveTest>BaseComputeServiceLiveTest.testConcurrentUseOfComputeServiceToCreateNodes:475 » Execution
Tests run: 118, Failures: 4, Errors: 0, Skipped: 19
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80#issuecomment-61739229
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1631](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1631/) 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/80#issuecomment-61733843
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by Adrian Cole <no...@github.com>.
> */
> @Delegate
> - @Path("/projects/{project}")
ok.. how about this. I'll revise the list javadoc when adding aggregate.
basically say, if you want to list across all zones go here?
p.s. I bet there's some nuance about aggregated list, like propagation
delay, right? Wondering if we should switch computeservice to use it or not.
p.p.s @nacx was working on a filter for images, and seems cleaner if there
were a cross-project aggregated image api. has that been requested?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80/files#r19846480
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by Adrian Cole <no...@github.com>.
> */
> @Delegate
> - @Path("/projects/{project}")
hmm option 3 considering there are a bunch of apis that support aggregation and they are all under the same path
https://www.googleapis.com/compute/v1/projects/project/aggregated
add AggregatedApi?
cc @andreaturli
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80/files#r19845447
Re: [jclouds-labs-google] Make instance api prettier. (#80)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #227](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/227/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/80#issuecomment-61734192