Client.APIAddresses

Client.APIAddresses

Expose the Server and Client sides of APIAddresses for a Client
(user). This is a step towards bug #1268470 (make api-endpoints use
the API rather than direct Provider access).

I wanted to propose this first, so we can discuss it. I think the
discussion is that new APIs should be on smaller facades than all of
Client, but I wasn't quite sure where to put this. If we have a
recognized spot for it, I'm happy to move the code over there.

https://code.launchpad.net/~jameinel/juju-core/api-addresses-1268470/+merge/202212

(do not edit description out of merge proposal)

[jameinel, 2014-01-19 12:19:35]

Please take a look.

[dimitern, 2014-01-20 11:41:04]

Looks good in general, but it might be worth to integrate this with juju/api.go
(connection caching), i.e. once we connect to the api and we're about to update
the cache, call APIAddresses() and save the up-to-date addresses in the cache.

https://codereview.appspot.com/54270043/diff/1/state/apiserver/client/api_tes...
File state/apiserver/client/api_test.go (right):

https://codereview.appspot.com/54270043/diff/1/state/apiserver/client/api_tes...
state/apiserver/client/api_test.go:277: m, err := s.State.AddMachine("quantal",
state.JobManageEnviron, state.JobManageState)
You could replace a bunch of lines here if you use
jujutesting.AddStateServerMachine, which does the same.

[jameinel, 2014-01-21 10:45:30]

On 2014/01/20 11:41:04, dimitern wrote:
> Looks good in general, but it might be worth to integrate this with
juju/api.go
> (connection caching), i.e. once we connect to the api and we're about to
update
> the cache, call APIAddresses() and save the up-to-date addresses in the cache.
> 
>
https://codereview.appspot.com/54270043/diff/1/state/apiserver/client/api_tes...
> File state/apiserver/client/api_test.go (right):
> 
>
https://codereview.appspot.com/54270043/diff/1/state/apiserver/client/api_tes...
> state/apiserver/client/api_test.go:277: m, err :=
s.State.AddMachine("quantal",
> state.JobManageEnviron, state.JobManageState)
> You could replace a bunch of lines here if you use
> jujutesting.AddStateServerMachine, which does the same.

I did check into this at Roger's suggestion. And State.APIAddresses() uses
State.stateAddresses() which explicitly requests the private addresses. Which
means we need to change the other bits (maybe we just put it behind querying the
Provider for an initial implementation.)

I'm thinking to rename it slightly, either ClientAPIAddresses or
PublicAPIAddresses, or something to make it clear that the values returned by
this function won't actually match the values from State.APIAddresses.
I'm also considering changing it to return a struct, so we can pass other stuff
in.

For now, I'll just keep investigating.