api(server)/uniter: Fix (result, bool, error)

api(server)/uniter: Fix (result, bool, error)

Change PublicAddress, PrivateAddress and CharmURL
to return (result, error) instead of (result, bool, error),
as discussed online, because since more than one
error can be returned the bool result is no longer
sufficient. Also, the first two calls where changed
at the server-side as well, while the last one
only at client-side for units, because for
services it has different semantics (the bool
means "forced" charm URL or not).

https://code.launchpad.net/~dimitern/juju-core/115-api-uniter-fix-result-ok-calls/+merge/183488

Requires: https://code.launchpad.net/~dimitern/juju-core/114-api-uniter-names-to-tags/+merge/183481

(do not edit description out of merge proposal)

[dimitern, 2013-09-02 15:22:09]

Please take a look.

[fwereade, 2013-09-02 15:28:12]

not quite there

https://codereview.appspot.com/13383046/diff/1/state/api/uniter/uniter_test.go
File state/api/uniter/uniter_test.go (left):

https://codereview.appspot.com/13383046/diff/1/state/api/uniter/uniter_test.g...
state/api/uniter/uniter_test.go:307: c.Assert(err, gc.IsNil)
This should have become a specific error, surely? ErrNoAddressAvailable or
similar.

https://codereview.appspot.com/13383046/diff/1/state/apiserver/uniter/uniter.go
File state/apiserver/uniter/uniter.go (left):

https://codereview.appspot.com/13383046/diff/1/state/apiserver/uniter/uniter....
state/apiserver/uniter/uniter.go:106: result.Results[i].Ok = ok
so:

if ok {
   set address 
} else {
   set appropriate err
}

across the board

[dimitern, 2013-09-02 15:53:11]

Please take a look.

https://codereview.appspot.com/13383046/diff/1/state/api/uniter/uniter_test.go
File state/api/uniter/uniter_test.go (left):

https://codereview.appspot.com/13383046/diff/1/state/api/uniter/uniter_test.g...
state/api/uniter/uniter_test.go:307: c.Assert(err, gc.IsNil)
On 2013/09/02 15:28:12, fwereade wrote:
> This should have become a specific error, surely? ErrNoAddressAvailable or
> similar.

Hmm, yes you're right. Done.

https://codereview.appspot.com/13383046/diff/1/state/apiserver/uniter/uniter.go
File state/apiserver/uniter/uniter.go (left):

https://codereview.appspot.com/13383046/diff/1/state/apiserver/uniter/uniter....
state/apiserver/uniter/uniter.go:106: result.Results[i].Ok = ok
On 2013/09/02 15:28:12, fwereade wrote:
> so:
> 
> if ok {
>    set address 
> } else {
>    set appropriate err
> }
> 
> across the board

Done.

[fwereade, 2013-09-02 16:14:18]

LGTM except one fix

https://codereview.appspot.com/13383046/diff/5001/state/api/uniter/uniter_tes...
File state/api/uniter/uniter_test.go (right):

https://codereview.appspot.com/13383046/diff/5001/state/api/uniter/uniter_tes...
state/api/uniter/uniter_test.go:363: c.Assert(err, gc.IsNil)
This should be an error.

[dimitern, 2013-09-02 16:40:50]

Please take a look.

https://codereview.appspot.com/13383046/diff/5001/state/api/uniter/uniter_tes...
File state/api/uniter/uniter_test.go (right):

https://codereview.appspot.com/13383046/diff/5001/state/api/uniter/uniter_tes...
state/api/uniter/uniter_test.go:363: c.Assert(err, gc.IsNil)
On 2013/09/02 16:14:18, fwereade wrote:
> This should be an error.

Done.