Merge lp://staging/~hduran-8/goose/testservice_errors into lp://staging/goose

Reviewers: mp+217818_code.launchpad.net,

Message:
Please take a look.

Description:
Added proper errortype in testservices.

Added an error type for testservers that
contains a message and an error code.
Switched novaservice to use new error type.

https://code.launchpad.net/~hduran-8/goose/testservice_errors/+merge/217818

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/99960043/

Affected files (+295, -58 lines):
   A [revision details]
   A testservices/errors.go
   A testservices/errors_test.go
   M testservices/novaservice/service.go
   M testservices/novaservice/service_test.go
   M testservices/service.go

https://codereview.appspot.com/99960043/diff/1/testservices/errors.go
File testservices/errors.go (right):

https://codereview.appspot.com/99960043/diff/1/testservices/errors.go#newcode5
testservices/errors.go:5: var nameReference = map[int]string{
Where do these values come from? I assume these are meant to match what
OpenStack returns exactly? If so, can you refer to some official
documentation?

If they're *not* meant to be identical to OpenStack, then this is
duplication of code in net/http.

https://codereview.appspot.com/99960043/diff/1/testservices/errors.go#newcode19
testservices/errors.go:19: type ServerError struct {
Does it perhaps make sense to expose more of goose/errors and use the
existing error types that would be returned by the real OpenStack?

https://codereview.appspot.com/99960043/diff/1/testservices/errors.go#newcode60
testservices/errors.go:60: // XXX: hduran-8 I infered this from the
python nova code, might be wrong
XXX is not typical to Juju or related projects. Use something like
"TODO(hduran-8) this is inferred from the Python nova code and might be
wrong; validate".

For most cases, we have a common format:
     TODO(nick) YYYY-MM-DD #bugid
     Summary.

and file a bug.

https://codereview.appspot.com/99960043/diff/1/testservices/errors.go#newcode66
testservices/errors.go:66: return &ServerError{
I would suggest a minor refactoring to simplify all of these functions:

func serverErrorf(code int, message string, args ...interface{})
*ServerError {
     return &ServerError{code: code, message: fmt.Sprintf(message,
args...)}
}

func NewAddFlavorError(id string) *ServerError {
     return serverErrorf(409, "A flavor with id %q already exists", id)
}

...

https://codereview.appspot.com/99960043/diff/1/testservices/errors.go#newcode67
testservices/errors.go:67: message: fmt.Sprintf("A flavor with id %q
already exists", id),
Do these error messages come from OpenStack, or did you make them up? We
typically start all error messages lower-case.

https://codereview.appspot.com/99960043/diff/1/testservices/errors_test.go
File testservices/errors_test.go (right):

https://codereview.appspot.com/99960043/diff/1/testservices/errors_test.go#newcode20
testservices/errors_test.go:20: // _, ok := err.(*error)
delete me

https://codereview.appspot.com/99960043/diff/1/testservices/errors_test.go#newcode30
testservices/errors_test.go:30: // _, ok := err.(*error)
delete me

https://codereview.appspot.com/99960043/

Please take a look.

https://codereview.appspot.com/99960043/

LGTM with an added comment

https://codereview.appspot.com/99960043/diff/20001/testservices/errors.go
File testservices/errors.go (right):

https://codereview.appspot.com/99960043/diff/20001/testservices/errors.go#newcode5
testservices/errors.go:5: var nameReference = map[int]string{
Can you please add a note about the origins of this mapping as
discussed?

https://codereview.appspot.com/99960043/

Please take a look.

https://codereview.appspot.com/99960043/

The attempt to merge lp:~hduran-8/goose/testservice_errors into lp:goose failed. Below is the output from the failed tests.

ok launchpad.net/goose 0.012s
ok launchpad.net/goose/client 0.109s
ok launchpad.net/goose/errors 0.011s
ok launchpad.net/goose/glance 0.019s
ok launchpad.net/goose/http 0.070s
ok launchpad.net/goose/identity 0.025s
ok launchpad.net/goose/nova 0.109s
ok launchpad.net/goose/swift 0.057s
ok launchpad.net/goose/sync 0.008s
ok launchpad.net/goose/testing/envsuite 0.010s
ok launchpad.net/goose/testing/httpsuite 0.063s
ok launchpad.net/goose/testservices 0.024s
? launchpad.net/goose/testservices/cmd [no test files]
ok launchpad.net/goose/testservices/hook 0.010s
ok launchpad.net/goose/testservices/identityservice 0.025s
ok launchpad.net/goose/testservices/novaservice 0.093s
? launchpad.net/goose/testservices/openstackservice [no test files]
ok launchpad.net/goose/testservices/swiftservice 0.054s
FAIL launchpad.net/goose/tools/secgroup-delete-all [build failed]

Setting GOPATH to: /home/tarmac/goose-trees
Running: go fmt ./...
Running: go build ./...
Running: go test ./...
# launchpad.net/goose/tools/secgroup-delete-all_test
tools/secgroup-delete-all/main_test.go:13: can't find import: "launchpad.net/goose/tools/secgroup-delete-all"
FAIL: failed running: go test ./...