[RFR] Rackspace Auto Scale: policies

[bison]

Adds a package for Rackspace Auto Scale policies.

[coveralls]

Coverage increased (+0.1%) to 80.179% when pulling 4d1a9a6 on bison:autoscale-policies into a09b5b4 on rackspace:master.

[bison]

Change, ChangePercent, and DesiredCapacity are empty interfaces here because a policy can only have one of the three. The API will return whichever field is appropriate and omit the others. In reality, ChangePercent is a float and the other two are integers.

So, this lets you test if each is nil, though everything comes back as a float64. I could add an UnmarshalJSON method that converts to integers as needed. Is that necessary? Or is this off base altogether?

[jrperritt]

I think you're on the right track here. I'd add an additional unmarshaling step: Define the Policy you have here as an unexported type (maybe in the commonExtractPolicies function as policy), decode the raw response data into that structure, and then add logic to massage it into a more user-friendly, exported Policy object like:

type Policy struct {
//...
AdjustmentType AdjustmentType
AdjustmentValue float64
//...
}
``

[bison]

Thinking about this more. Not sure this is doable with mapstructure. It's easy enough adding an UnmarshalJSON method to Policy, but I think that means doing one of the following:

• Keep the raw HTTP response in the Body field on the gophercloud.Result structs and do the decoding in the extract methods.
• Do the decoding in the request methods, and store a Policy in gophercloud.Result.Body. The extract methods don't do much then.
• Add a Policy to the policyResult struct and store the decoded policies there.

The first option seems most reasonable to me. Anyway, my hangup is that all of those break from the norm of decoding into a generic map on gophercloud.Result.Body and then using mapstructure. I can't find anywhere else in the code that does something like this.

Thoughts? Also, if it's easier, I can push something I have along the lines of the first option above and we can talk about it more concretely maybe.

EDIT: Maybe I misunderstood what you said. I guess you mean use mapstructure to decode into the unexported type then just define a helper to build a Policy from that. Is that right? If so, I'm guessing that's preferable to what I described?

[jrperritt]

I guess you mean use mapstructure to decode into the unexported type then just define a helper to build a Policy from that. Is that right?

Exactly. Doesn't have to be a separate function; could be defined in commonExtractPolicies.

[bison]

Makes sense. Done.

[coveralls]

Coverage increased (+0.1%) to 80.207% when pulling 92d20ec on bison:autoscale-policies into a09b5b4 on rackspace:master.

[jrperritt]

since only one of these can be set, we can else if here

[bison]

Sure. Done.

[coveralls]

Coverage increased (+0.1%) to 80.19% when pulling eedc8d9 on bison:autoscale-policies into a09b5b4 on rackspace:master.

[jrperritt]

Looks great. Let me know when you're done making changes and I'll merge it.

[coveralls]

Coverage increased (+0.06%) to 80.127% when pulling 0310e5c on bison:autoscale-policies into a09b5b4 on rackspace:master.

[bison]

Pushed 0310e5c with the ScheduleArgs interface. It's not finished. I haven't changed the create and update bits, but this works for getting / listing policies. Switching on the schedule type in those cases would look like this:

policy, err := policies.Get(client, group, policyID).Extract()

switch s := policy.Schedule.(type) {
case policies.At:
        t := time.Time(s)
        fmt.Println("EXECUTE AT:", t.Format(time.RFC850))

case policies.Cron:
        fmt.Println("CRON:", s)

default:
        fmt.Println("Webhook.")
}

Creating policies with an At schedule seems nice with this because you can use all the standard time manipulation stuff and then create the At from that. Users shouldn't have to worry about the ToPolicyArgs() method. The methods for converting to maps for creates and updates can handle that.

[bison]

Added the ScheduleArgs bits for create and update operations. Examples in the tests.

I think this is finished assuming you're good with the ScheduleArgs thing. If not, let me know and we can work out another way.

[coveralls]

Coverage increased (+0.1%) to 80.177% when pulling a06e2ca on bison:autoscale-policies into a09b5b4 on rackspace:master.

[jrperritt]

Maybe we can compromise and just have AtSchedule and CronSchedule fields? After thinking about it, I don't think we should force users to switch (neither on type nor value) to figure out what's there.

[bison]

We can do that. My only worry is the zero values. I really think AtSchedule works best as a time.Time, but if I request a policy configured with a cron schedule, I don't want AtSchedule set to 0001-01-01 00:00:00 +0000 UTC... and in the opposite case, even having Cron be an empty string is a little strange.

We could make those fields interface{}, so they can default to nil, but that's not great either and it seems like users will have to do some checking to see what's what in any case.

[bison]

Or did you mean keep the ScheduleArgs interface, but just have two fields and do some type validation before creates / updates?

[jrperritt]

I'm very much a fan of the ScheduleArgs in the CreateOpts and UpdateOpts; I think that works well. I'm trying to figure out if there's a way for users to get at that information in a response object without having to type-assert. Though I agree zero-values may not be the best solution either.

[bison]

We could keep the ScheduleArgs interface and the two types for create and updates, and revert to Args map[string]string for Policy. If you request a policy with an at argument, then need to update it, you can always use time.Parse to turn it into a policies.At.

That doesn't seem ideal, and you still have to do some looking into the map, but maybe it's more straight forward than the type switch. I'm not really sure.

[jrperritt]

No, let's go with what you have now. I think it's the best of the options I can think of right now.

[jrperritt]

Is it ready to merge?

[bison]

Yeah, I think it's good.

[jrperritt]

+2