api: allow shorter dialing time

api: allow shorter dialing time

We want to allow a shorter timeout when dialling the API
server.

https://code.launchpad.net/~rogpeppe/juju-core/315-api-dialopts/+merge/165744

Requires: https://code.launchpad.net/~rogpeppe/juju-core/314-worker-fix-runner-deadlock/+merge/165741

(do not edit description out of merge proposal)

[rog, 2013-05-25 13:40:29]

Please take a look.

[fwereade, 2013-05-27 20:38:08]

LGTM

[jameinel, 2013-05-29 08:31:26]

I realize you want the ability to configure it during the test suite. However if
you were actually using this, wouldn't it be easier to pass 'nil' to mean use
the default rather than having to figure out where the DefaultDialOpts function
is and call it?

Anyway, in cases like this where you don't have a great zero struct, it seems
valid to take a pointer so callers can just pass nil and the function can set
valid defaults for the parameter.

otherwise, LGTM.

[Kiran_Hyd, 2013-05-29 11:49:17]

Please review

https://codereview.appspot.com/9771043/diff/2001/environs/agent/agent.go
File environs/agent/agent.go (right):

https://codereview.appspot.com/9771043/diff/2001/environs/agent/agent.go#newc...
environs/agent/agent.go:208: func (c *Conf) OpenAPI(dialOpts api.DialOpts) (st
*api.State, newPassword string, err error) {
Kiran Test Comment 2

https://codereview.appspot.com/9771043/diff/2001/environs/agent/agent.go#newc...
environs/agent/agent.go:224: st, err = api.Open(&info, dialOpts)
Kiran Test Comment

[Kiran_Hyd, 2013-05-29 11:51:21]

Fixed

https://codereview.appspot.com/9771043/diff/2001/environs/agent/agent.go
File environs/agent/agent.go (right):

https://codereview.appspot.com/9771043/diff/2001/environs/agent/agent.go#newc...
environs/agent/agent.go:208: func (c *Conf) OpenAPI(dialOpts api.DialOpts) (st
*api.State, newPassword string, err error) {
On 2013/05/29 11:49:18, kiran9866650520 wrote:
> Kiran Test Comment 2

Done.

https://codereview.appspot.com/9771043/diff/2001/environs/agent/agent.go#newc...
environs/agent/agent.go:224: st, err = api.Open(&info, dialOpts)
On 2013/05/29 11:49:18, kiran9866650520 wrote:
> Kiran Test Comment

Done.

[rog, 2013-06-07 17:52:35]

On 29 May 2013 09:31,  <john.meinel@canonical.com> wrote:
> I realize you want the ability to configure it during the test suite.
> However if you were actually using this, wouldn't it be easier to pass
> 'nil' to mean use the default rather than having to figure out where the
> DefaultDialOpts function is and call it?
>
> Anyway, in cases like this where you don't have a great zero struct, it
> seems valid to take a pointer so callers can just pass nil and the
> function can set valid defaults for the parameter.
>
> otherwise, LGTM.
>
> https://codereview.appspot.com/9771043/

I've just copied the existing state API for this.

I think it's reasonable actually - the behaviour is quite
obvious, and it's easy for the user to choose their
own timeouts or inspect the default values without
reading the state source code.
(I'd actually prefer DefaultDialOpts to be a variable rather than a function
so you could read the default values in the docs, but
I think copying the state API exactly is more important here)

I may change my mind tomorrow though!

[rog, 2013-06-25 06:02:41]

merged as https://codereview.appspot.com/10415043/

[rog, 2013-06-25 06:04:30]

On 2013/06/25 06:02:41, rog wrote:
> merged as https://codereview.appspot.com/10415043/