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/12/05 20:27:50 UTC
[jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
This adds coverage of TargetPoolApi:AggregatedList and TargetPoolApi:GetHealth.
Also updates HttpHealthChecks. Defaults were not being set in HttpHealthChecks. This was hidden by the fact that we assumed default values when nothing was returned in the request. This PR addresses this and changes HttpHealthCheckCreationOptions to enable making a request with or without the default values.
Also converts TargetPoolApi to mock test.
Turns out a force push to squash a comment edit made it so I could not reopen PR #105
You can merge this Pull Request by running:
git pull https://github.com/GoogleCloudPlatform/jclouds-labs-google targetPoolUpdate
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/108
-- Commit Summary --
* Added targetPool:AggregatedList, getHealth. Updated HttpHealthCheck
-- File Changes --
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/binders/HttpHealthCheckCreationBinder.java (4)
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/HealthStatus.java (18)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/HttpHealthCheck.java (24)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/AggregatedListApi.java (51)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/BackendServiceApi.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/HttpHealthCheckApi.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/TargetPoolApi.java (17)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/options/HttpHealthCheckCreationOptions.java (280)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/binders/HttpHealthCheckCreationBinderTest.java (16)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/AggregatedListApiLiveTest.java (18)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/AggregatedListApiMockTest.java (34)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/BackendServiceApiMockTest.java (6)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/HttpHealthCheckApiExpectTest.java (4)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/HttpHealthCheckApiLiveTest.java (43)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/HttpHealthCheckApiMockTest.java (8)
D google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/TargetPoolApiExpectTest.java (263)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/TargetPoolApiLiveTest.java (73)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/TargetPoolApiMockTest.java (178)
R google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseHealthStatusTest.java (18)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseTargetPoolListTest.java (7)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseTargetPoolTest.java (9)
A google-compute-engine/src/test/resources/aggregated_target_pool_list.json (46)
A google-compute-engine/src/test/resources/aggregated_target_pool_list_empty.json (43)
R google-compute-engine/src/test/resources/health_status_get_health.json (2)
M google-compute-engine/src/test/resources/httphealthcheck_insert.json (2)
A google-compute-engine/src/test/resources/httphealthcheck_patch.json (1)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/108.patch
https://github.com/jclouds/jclouds-labs-google/pull/108.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1842](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1842/) 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/108#issuecomment-65842026
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -157,6 +158,22 @@
> @Nullable
> Operation removeHealthCheck(@PathParam("targetPool") String targetPool, @PayloadParam("healthChecks") List<URI> healthChecks);
>
> + /**
> + * Gets the HealthStatus of an instance in a targetPool.
> + *
> + * @param targetPool the name of the target pool.
> + * @param healthChecks the self-links of the health checks to be removed from the targetPool.
> + *
> + * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to
> + * you, and look for the status field.
> + */
> + @Named("TargetPools:getHealth")
> + @POST
> + @Path("/{targetPool}/getHealth")
> + @Nullable
Usually, 404 exceptions are propagated as a `ResourceNotFoundException` unless a fallback is configured to transform that into a `null` return value. Can this method really return null?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108/files#r21492545
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by danbroudy <no...@github.com>.
I do have a few more GCE tasks in the works. Getting all the live tests to pass is probably at the top of that list. As for the SecurityGroupExtension, I haven't really looked at it and don't know much about what it does.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108#issuecomment-66361373
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by Ignasi Barrera <no...@github.com>.
> + * have a workaround for.
> + */
> + /**
> + * @param requestPath Defaults to "/" when null.
> + * @param port Defaults to 80 when null.
> + * @param checkIntervalSec Defaults to 5 when null.
> + * @param timeoutSec Defaults to 5 when null.
> + * @param unhealthyThreshold Defaults to 2 when null.
> + * @param healthyThreshold Defaults to 2 when null.
> + */
> + static HttpHealthCheckCreationOptions createWithDefaults(String host,
> + String requestPath, Integer port, Integer checkIntervalSec, Integer timeoutSec, Integer unhealthyThreshold,
> + Integer healthyThreshold, String description) {
> + return create(host, requestPath != null ? requestPath : "/", port != null ? port : 80,
> + checkIntervalSec != null ? checkIntervalSec : 5, timeoutSec != null ? timeoutSec : 5,
> + unhealthyThreshold != null ? unhealthyThreshold : 2, healthyThreshold != null ? healthyThreshold : 2, description);
Would it make sense to move these default to constant values or to a constant class?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108/files#r21492752
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1843](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1843/) 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/108#issuecomment-65843509
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by danbroudy <no...@github.com>.
After looking at it again i found that the Javadocs `@value` doesn't appear to render correctly with `static final Integer` but does render correctly with `static final int`.
Ill update this in an upcoming PR.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108#issuecomment-66357052
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by Ignasi Barrera <no...@github.com>.
Let's focus on those tasks then. Regarding the live tests, I'll give them a run and help with them :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108#issuecomment-66361638
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for cleaning up all the `@Nullable` methods. Squashed and pushed to master as [672b942](https://git-wip-us.apache.org/repos/asf?p=jclouds-labs-google.git;a=commit;h=672b94269038f5fd6a3ddbd5514ff7cd9b7b6cac).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108#issuecomment-66355225
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by danbroudy <no...@github.com>.
> @@ -157,6 +158,22 @@
> @Nullable
> Operation removeHealthCheck(@PathParam("targetPool") String targetPool, @PayloadParam("healthChecks") List<URI> healthChecks);
>
> + /**
> + * Gets the HealthStatus of an instance in a targetPool.
> + *
> + * @param targetPool the name of the target pool.
> + * @param healthChecks the self-links of the health checks to be removed from the targetPool.
> + *
> + * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to
> + * you, and look for the status field.
> + */
> + @Named("TargetPools:getHealth")
> + @POST
> + @Path("/{targetPool}/getHealth")
> + @Nullable
I think you are correct. This means that add/remove healthCheck and add/remove instance are not nullable either.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108/files#r21549411
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by danbroudy <no...@github.com>.
Im not sure if i did the javadoc {@value #STATIC_FIELD} correctly. I have never done that before.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108#issuecomment-66337864
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by Ignasi Barrera <no...@github.com>.
Neither do I :) But from what I've read it is OK. The value will be rendered as long as the referenced fields are initialised with a literal value.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108#issuecomment-66352115
Re: [jclouds-labs-google] Added targetPool:AggregatedList, getHealth.
Updated HttpHealthCheck (#108)
Posted by Ignasi Barrera <no...@github.com>.
Cool. If you run the build with the `-Pdoc` profile, the javadocs will be generated and you'll be able to see how they are rendered. Thanks!
Off-topic: do you have many tasks in the backlog for GCE? If not, I'd like to discuss what to do with the SecurityGroupExtension. It should be refactored or removed, but in its current status it does not do what it is supposed to do :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/108#issuecomment-66357619