Code review comment for lp:~axwalk/goose/nova-availability-zones

Revision history for this message
Martin Packman (gz) wrote :

LGTM.

https://codereview.appspot.com/103900045/diff/20001/nova/local_test.go
File nova/local_test.go (right):

https://codereview.appspot.com/103900045/diff/20001/nova/local_test.go#newcode231
nova/local_test.go:231: s.openstack.Nova.SetAvailabilityZones()
This is pretty magical test server behaviour, but you have a nice big
comment at least.

https://codereview.appspot.com/103900045/diff/20001/nova/nova.go
File nova/nova.go (right):

https://codereview.appspot.com/103900045/diff/20001/nova/nova.go#newcode695
nova/nova.go:695: return nil, nil
I'd prefer we return a specific error that can be checked, but that's
not really a job for this proposal, we need better goose-wide handling
of extensions and errors.

https://codereview.appspot.com/103900045/diff/20001/testservices/novaservice/service_http.go
File testservices/novaservice/service_http.go (right):

https://codereview.appspot.com/103900045/diff/20001/testservices/novaservice/service_http.go#newcode228
testservices/novaservice/service_http.go:228:
errAvailabilityZoneIsNotAvailable = &errorResponse{
Rather than doing this the old way, you should use the new
testservices/errors.go method of creating the error.

https://codereview.appspot.com/103900045/

« Back to merge proposal