Code review comment for lp:~thumper/juju-core/api-juju-run

https://codereview.appspot.com/49960045/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/49960045/diff/1/cmd/jujud/machine.go#newcode310
cmd/jujud/machine.go:310: })
On 2014/01/10 03:19:03, wallyworld wrote:
> This is clumsy - port, cert, key all are derived from agentConfig, aka
> a.Conf.config

> So we should include dataDir with the result of APIServerDetails and
pass that
> in

Done.

https://codereview.appspot.com/49960045/diff/1/provider/dummy/environs.go
File provider/dummy/environs.go (right):

https://codereview.appspot.com/49960045/diff/1/provider/dummy/environs.go#newcode601
provider/dummy/environs.go:601: estate.apiServer, err =
apiserver.NewServer(st, "localhost:0", []byte(testing.ServerCert),
[]byte(testing.ServerKey), config)
On 2014/01/10 03:19:03, wallyworld wrote:
> As per my other comment, just pass in dataDir

Done.

https://codereview.appspot.com/49960045/diff/1/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/49960045/diff/1/state/api/client.go#newcode417
state/api/client.go:417:
On 2014/01/10 03:19:03, wallyworld wrote:
> The above structs should be in params/internal.go right?

Is there a standard? This needs to be accessed by both the api caller
and the apiserver. If internal is the place to put them, I'll move it,
but it wasn't clear when writing that they should go there.

https://codereview.appspot.com/49960045/diff/1/state/apiserver/apiserver.go
File state/apiserver/apiserver.go (right):

https://codereview.appspot.com/49960045/diff/1/state/apiserver/apiserver.go#newcode33
state/apiserver/apiserver.go:33: agentConfig agent.Config
On 2014/01/10 03:19:03, wallyworld wrote:
> Just dataDir would be better I think

Done.

https://codereview.appspot.com/49960045/diff/1/state/apiserver/client/run.go
File state/apiserver/client/run.go (right):

https://codereview.appspot.com/49960045/diff/1/state/apiserver/client/run.go#newcode168
state/apiserver/client/run.go:168: outstanding.Wait()
On 2014/01/10 03:19:03, wallyworld wrote:
> Should there be some sort of timeout? Will this hang if an individual
command
> hangs?

It will wait based on the timeout passed through to
ExecuteCommandOnMachine.

https://codereview.appspot.com/49960045/