Goose API tweaks

Goose API tweaks

The Goose API has been tweaked. This branch ensures juju-core's OpenStack provider is compatible.

https://code.launchpad.net/~wallyworld/juju-core/goose-api-catchup/+merge/139378

(do not edit description out of merge proposal)

[wallyworld, 2012-12-12 03:39:16]

Please take a look.

[wallyworld, 2012-12-12 04:59:06]

Please take a look.

[jameinel, 2012-12-12 09:09:34]

LGTM, assuming this must land at the appropriate time as updates to goose.

[dave_cheney.net, 2012-12-17 07:26:00]

LGTM. Thank you.

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/config.go
File environs/openstack/config.go (right):

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/config.go...
environs/openstack/config.go:81: missingAttributef := "required environment
variable not set for credentials attribute: %s"
i normally just say
   format := "the format string"

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/openstack.go
File environs/openstack/openstack.go (right):

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/openstack...
environs/openstack/openstack.go:7: defaultImageId  =
"0f602ea9-c09e-440c-9e29-cfae5635afa3"
possibly a comment here to say where this value came from (i'm assuming some
internal openstack instance), in the event that image lookup gets pushed back.

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/provider....
environs/openstack/provider.go:248: var server *nova.Entity
please move this closer to line 250

[wallyworld, 2012-12-18 01:27:06]

Please take a look.

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/config.go
File environs/openstack/config.go (right):

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/config.go...
environs/openstack/config.go:81: missingAttributef := "required environment
variable not set for credentials attribute: %s"
On 2012/12/17 07:26:00, dfc wrote:
> i normally just say
>    format := "the format string"

Done.

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/openstack.go
File environs/openstack/openstack.go (right):

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/openstack...
environs/openstack/openstack.go:7: defaultImageId  =
"0f602ea9-c09e-440c-9e29-cfae5635afa3"
On 2012/12/17 07:26:00, dfc wrote:
> possibly a comment here to say where this value came from (i'm assuming some
> internal openstack instance), in the event that image lookup gets pushed back.

Done.

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/6929055/diff/1009/environs/openstack/provider....
environs/openstack/provider.go:248: var server *nova.Entity
On 2012/12/17 07:26:00, dfc wrote:
> please move this closer to line 250

Done.

[dave_cheney.net, 2012-12-18 02:10:34]

Still LGTM.

https://codereview.appspot.com/6929055/diff/9001/environs/openstack/config.go
File environs/openstack/config.go (right):

https://codereview.appspot.com/6929055/diff/9001/environs/openstack/config.go...
environs/openstack/config.go:81: format := "required environment variable not
set for credentials attribute: %s"
You can also use an anon function here

var err := func(s string) string { return fmt.Errrof(..., s) }

then

return nil, err("User")

[fwereade, 2012-12-18 14:45:39]

A bit baffled by this: I can't spot the differences with
https://codereview.appspot.com/6923056/ so please assume that my comments from
there apply to whichever branch is the original.

[wallyworld, 2012-12-19 07:02:24]

*** Submitted:

Goose API tweaks

The Goose API has been tweaked. This branch ensures juju-core's OpenStack
provider is compatible.

R=jameinel, dfc, fwereade
CC=
https://codereview.appspot.com/6929055