Merge lp://staging/~wallyworld/goose/hpopenstack into lp://staging/goose

Quick note, it might be easier to review and land if we split out the "regionMatches" component from the rest of it.
However, I'll try to review it all together for now.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2013-02-07 15:04, Ian Booth wrote:
> Ian Booth has proposed merging lp:~wallyworld/goose/hpopenstack
> into lp:goose.
>
> Requested reviews: The Go Language Gophers (gophers)
>
> For more details, see:
> https://code.launchpad.net/~wallyworld/goose/hpopenstack/+merge/147069
>
> Add HP Cloud support
>
> A few major changes here. HP Cloud sends many core data structures
> over the wire with int IDs rather than string. It also lacks a
> security groups API but instead adds the groups to the server
> detail record.
>
> To make everything work in the least invasive way, I added custom
> json (un)marshallers to the affected data structures. The affected
> data structures are Entity, ServerDetail, FloatingIP, FlavorDetail.
> The approach allows the built in marshalling to be used for
> everything except the IDs, which are converted from int or string
> as required.
>
> The live tests are enhanced to allow them to be run against HP
> Cloud or Canonistack as required: go test -gocheck.v -live -vendor
> canonistack or go test -gocheck.v -live -vendor hpcloud
>
> The local tests are configured to run twice - once with nemric ids
> and one with string ids.
>

1) Shouldn't there be direct tests (unit tests) of regionMatches?

2) useNumericIds seems like it should be in testservices/novaservice
rather than in nova/

I realize it is used in "convertId", but I would have thought we would
standardize that at the abstraction layer above goose, Id is always a
string (since it is sometimes a string). So while it might be an int
or a string on the wire, in memory we would just store it as a string.

Is it something where we have to pass it back to HP/Canonistack as the
same type? If so, then it sounds like something that would need to be
passed around, rather than a global state switch. (When we read it, if
we got an int, pass it back as an int.)

3) Could you have done this more simply with:

type Entity struct {
  Id string `json:"-"`
  GenericId interface{} `json:"id"`
  UUID string `json:"uuid"`
  Links []Link `json:"links"`
  Name string `json:"name"`
}

And then instead of having an UnmarshalJSON function, you can
unmarshal as normal, and just map it into the in-memory struct.

Combining it with (2) you might have:
type Entity struct {
  Id string `json:"-"`
  GenericId interface{} `json:"id"`
  IdWasInt bool `json:"-"`
  UUID string `json:"uuid"`
  Links []Link `json:"links"`
  Name string `json:"name"`
}

Or maybe another option is a new type for Id?

type GenericId interface{}

func (g *GenericId) String() {
}

func (g *GenericId) AsInt() {
}

And then we store it in memory as whatever we were given by JSON, but
we use String() to think about it.

I'm certainly not blocking on this, and it may be something you tried
but it was easier to do with custom UnmarshalJSON functions. You just
had to add an awful lot of them, and I was hoping we could avoid that.

677 +type JsonFloatingIP struct {
678 + FloatingIP `json:"-"`
679 +}
680 +

I'd like us to standardize on using JSON instead of Json (same with
HTTP and URL) in variable and class names. We've done that in some
places (though not all). It cert...

I've reworked the JSON serialisation implementation to significantly simplify it. By using type aliases, I can work directly on the API data types and totally eliminate the need to change any nova or test service code. The serialisation is moved to a separate module and so effectively can be thought of as orthogonal to the behaviour of the core business logic.