You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Zack Shoylev <no...@github.com> on 2014/01/03 01:45:10 UTC
[jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy type
(#71)
You can merge this Pull Request by running:
git pull https://github.com/rackerlabs/jclouds-labs-openstack add-scheduling-policy
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/71
-- Commit Summary --
* JCLOUDS-418 Add schedule Scaling Policy type
-- File Changes --
M rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/ScalingPolicy.java (66)
M rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/domain/ScalingPolicyResponse.java (7)
M rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/functions/ParseGroupResponse.java (1)
M rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/functions/ParseScalingPoliciesResponse.java (1)
M rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/functions/ParseScalingPolicyResponse.java (1)
M rackspace-autoscale/src/main/java/org/jclouds/rackspace/autoscale/v1/internal/ParseHelper.java (5)
M rackspace-autoscale/src/test/java/org/jclouds/rackspace/autoscale/v1/features/ScalingPolicyApiLiveTest.java (52)
M rackspace-autoscale/src/test/java/org/jclouds/rackspace/autoscale/v1/features/ScalingPolicyApiMockTest.java (92)
M rackspace-autoscale/src/test/java/org/jclouds/rackspace/autoscale/v1/features/WebhookApiLiveTest.java (20)
A rackspace-autoscale/src/test/resources/autoscale_policy_schedule_at_create_request.json (11)
A rackspace-autoscale/src/test/resources/autoscale_policy_schedule_at_create_response.json (20)
A rackspace-autoscale/src/test/resources/autoscale_policy_schedule_cron_create_request.json (11)
A rackspace-autoscale/src/test/resources/autoscale_policy_schedule_cron_create_response.json (20)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/71.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/71.diff
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Zack Shoylev <no...@github.com>.
After talking to @everett-toews - will refactor to completely hide the map by adding 2 getters and an enum type for schedule.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31691510
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #136](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/136/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31554242
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Zack Shoylev <no...@github.com>.
https://issues.apache.org/jira/browse/JCLOUDS-418
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31498005
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Everett Toews <no...@github.com>.
> + * @return The builder object.
> + * @see ScalingPolicyTargetType
> + * @see ScalingPolicy#getTargetType()
> + * @see <a href="http://en.wikipedia.org/wiki/Cron">Cron</a>
> + */
> + public Builder cronSchedule(String cron) {
> + this.type = ScalingPolicyType.SCHEDULE;
> + this.args = Maps.newHashMap();
> + args.put("cron", cron);
> + return this;
> + }
> +
> + /**
> + * @param at This parameter specifies the time at which this policy will be executed.
> + * This property is mutually exclusive with the "cron" parameter.
> + * You can either provide "cron" or "at" for a given policy, but not both.
This disclaimer should also be a comment in cronSchedule()
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8638418
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Zack Shoylev <no...@github.com>.
> @@ -195,12 +211,45 @@ public Builder targetType(ScalingPolicyTargetType targetType) {
> this.targetType = targetType;
> return this;
> }
> +
> + /**
> + * @param cron This parameter specifies the recurring time when the policy will be executed as a cron entry.
> + * For example, if this is parameter is set to "1 0 * * *",
> + * the policy will be executed at one minute past midnight (00:01)
> + * every day of the month, and every day of the week.
> + * @return The builder object.
> + * @see ScalingPolicyTargetType
> + * @see ScalingPolicy#getTargetType()
> + * @see <a href="http://en.wikipedia.org/wiki/Cron">Cron</a>
> + */
> + public Builder cronSchedule(String cron) {
> + this.type = ScalingPolicyType.SCHEDULE;
> + this.args = Maps.newHashMap();
Yes, this ensures either/or. Yes, this should be an ImmutableMap...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8644272
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Andrew Phillips <no...@github.com>.
> @@ -112,7 +125,8 @@ public boolean equals(Object obj) {
> Objects.equal(this.type, that.type) &&
> Objects.equal(this.cooldown, that.cooldown) &&
> Objects.equal(this.target, that.target) &&
> - Objects.equal(this.targetType, that.targetType);
> + Objects.equal(this.targetType, that.targetType) &&
> + Objects.equal(this.args, that.args);
[minor] double space after the comma
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8630898
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #752](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/752/) 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-openstack/pull/71#issuecomment-31738991
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Zack Shoylev <no...@github.com>.
> I think you need a ScalingPolicyScheduleType enum. You've got enums for everything else.
What do you have in mind exactly? Something like:
.schedule(ScalingPolicyScheduleType.AT, "some date") ?
and conversely:
.getScheduleType()
.getScheduleString()
?
I actually think that would be more complex (regardless of consistency here). The reason I have used enums was because I couldn't figure out a simpler way to make it easier for the user.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31551177
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Zack Shoylev <no...@github.com>.
@everett-toews - I think this is what we discussed. If i didn't miss anything, I am ready to squash/rebase/merge. Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31697605
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Andrew Phillips <no...@github.com>.
Looks good to me, but interested to see if @everett-toews has some more comments. Squash'n'rebase too, obviously
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31689929
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Everett Toews <no...@github.com>.
I think you need a `ScalingPolicyScheduleType` enum. You've got enums for everything else.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31537274
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Andrew Phillips <no...@github.com>.
> @@ -195,12 +211,45 @@ public Builder targetType(ScalingPolicyTargetType targetType) {
> this.targetType = targetType;
> return this;
> }
> +
> + /**
> + * @param cron This parameter specifies the recurring time when the policy will be executed as a cron entry.
> + * For example, if this is parameter is set to "1 0 * * *",
> + * the policy will be executed at one minute past midnight (00:01)
> + * every day of the month, and every day of the week.
> + * @return The builder object.
> + * @see ScalingPolicyTargetType
> + * @see ScalingPolicy#getTargetType()
> + * @see <a href="http://en.wikipedia.org/wiki/Cron">Cron</a>
> + */
> + public Builder cronSchedule(String cron) {
> + this.type = ScalingPolicyType.SCHEDULE;
> + this.args = Maps.newHashMap();
Any reason not to move this into the declaration of the variable above? Too expensive, or..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8630924
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Andrew Phillips <no...@github.com>.
> @@ -96,6 +96,11 @@
> } else {
> scalingPolicyMapBuilder.put(scalingPolicy.getTargetType().toString(), targetString);
> }
> +
> + if(scalingPolicy.getSchedulingArgs() != null
Space after `if`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8630969
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-openstack #750](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/750/) 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-openstack/pull/71#issuecomment-31695530
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Andrew Phillips <no...@github.com>.
> @@ -132,6 +132,7 @@ public Group apply(HttpResponse from) {
> ((Double)scalingPolicyMap.get("cooldown")).intValue(),
> DoubleMath.isMathematicalInteger(d) ? Integer.toString(d.intValue()) : Double.toString(d),
> targetType,
> + (Map<String, String>)scalingPolicyMap.get("args"),
Space before `scalingPolicyMap`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8630953
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #138](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/138/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31695751
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #135](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/135/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31498304
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Zack Shoylev <no...@github.com>.
> + * @return The builder object.
> + * @see ScalingPolicyTargetType
> + * @see ScalingPolicy#getTargetType()
> + * @see <a href="http://en.wikipedia.org/wiki/Cron">Cron</a>
> + */
> + public Builder cronSchedule(String cron) {
> + this.type = ScalingPolicyType.SCHEDULE;
> + this.args = Maps.newHashMap();
> + args.put("cron", cron);
> + return this;
> + }
> +
> + /**
> + * @param at This parameter specifies the time at which this policy will be executed.
> + * This property is mutually exclusive with the "cron" parameter.
> + * You can either provide "cron" or "at" for a given policy, but not both.
Done :) Note: The javadoc here is mostly verbatim from the API docs.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8645053
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Andrew Phillips <no...@github.com>.
> @@ -0,0 +1,11 @@
> +[
> + {
> + "args": {
> + "cron": "23 * * * *"
> + },
> + "changePercent": -5.5,
> + "cooldown": 2,
> + "name": "scale down by 5.5 percent at 11pm",
> + "type": "schedule"
> + }
[minor] indenting here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8630999
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Andrew Phillips <no...@github.com>.
> @@ -84,6 +84,7 @@
> ((Double)scalingPolicyMap.get("cooldown")).intValue(),
> DoubleMath.isMathematicalInteger(d) ? Integer.toString(d.intValue()) : Double.toString(d),
> targetType,
> + (Map<String, String>)scalingPolicyMap.get("args"),
Space before `scalingPolicyMap`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8630960
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Zack Shoylev <no...@github.com>.
Anything else? Or does it look ready. This will also have to be backported.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31660657
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Andrew Phillips <no...@github.com>.
> @@ -81,6 +81,7 @@ public ScalingPolicyResponse apply(HttpResponse from) {
> ((Double)scalingPolicyMap.get("cooldown")).intValue(),
> DoubleMath.isMathematicalInteger(d) ? Integer.toString(d.intValue()) : Double.toString(d),
> targetType,
> + (Map<String, String>)scalingPolicyMap.get("args"),
Space before `scalingPolicyMap`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8630961
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Andrew Phillips <no...@github.com>.
> @@ -204,12 +204,18 @@ public void testUpdateWebhook() {
> public void testGetWebhook() {
> for (String zone : api.getConfiguredZones()) {
> Group g = created.get(zone).get(0);
> - WebhookApi webhookApi = api.getWebhookApiForZoneAndGroupAndPolicy(zone, g.getId(), g.getScalingPolicies().iterator().next().getId());
> - WebhookResponse webhookList = webhookApi.list().first().get();
> - WebhookResponse webhookGet = webhookApi.get(webhookList.getId());
> - assertNotNull(webhookList);
> - assertNotNull(webhookGet);
> - assertEquals(webhookList, webhookGet);
> + WebhookApi webhookApi;
> + boolean foundWebhook = false;
> + for (ScalingPolicyResponse sp : g.getScalingPolicies()) {
> + webhookApi = api.getWebhookApiForZoneAndGroupAndPolicy(zone, g.getId(), sp.getId());
> + WebhookResponse webhookList = webhookApi.list().first().get();
Rename `webhookList` to `webhookResponse`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8630989
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by Andrew Phillips <no...@github.com>.
> @@ -96,6 +96,11 @@
> } else {
> scalingPolicyMapBuilder.put(scalingPolicy.getTargetType().toString(), targetString);
> }
> +
> + if(scalingPolicy.getSchedulingArgs() != null
> + && scalingPolicy.getType().equals(ScalingPolicy.ScalingPolicyType.SCHEDULE)) {
Static import `ScalingPolicy.ScalingPolicyType` or `ScalingPolicy.ScalingPolicyType.SCHEULDE`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71/files#r8630976
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #139](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/139/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31739101
Re: [jclouds-labs-openstack] JCLOUDS-418 Add schedule Scaling Policy
type (#71)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #137](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/137/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/71#issuecomment-31554366