environs/agent: new package

environs/agent: new package

For the moment, it only deals with settings, but some
functionality will probably move from environs/tools.go
into this package in a future CL.

I decided to write all the settings to a single file rather than the
current multiple files) because it's less code, more easily extensible,
and if we do want different goroutines in an agent to write different
aspects of the file, it would be straightforward to add some locking
around the write.

https://code.launchpad.net/~rogpeppe/juju-core/186-agent-package/+merge/142676

(do not edit description out of merge proposal)

[rog, 2013-01-10 12:38:28]

Please take a look.

[TheMue, 2013-01-10 12:57:18]

Mostly LGTM, only two little hints.

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

https://codereview.appspot.com/7064066/diff/2001/environs/agent/agent.go#newc...
environs/agent/agent.go:85: return fmt.Errorf("invalid server address %q", a)
Not sure about this test. It only checks the format in a very simple way. But
even if that is correct the server address may be invalid. So maybe the error
returned when trying to open an invalid one is enough and more helpful.

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

https://codereview.appspot.com/7064066/diff/2001/environs/agent/agent_test.go...
environs/agent/agent_test.go:128: // move the configuration file to a different
directory
"Move", just to be consistent. ;)

[rog, 2013-01-10 13:01:49]

Please take a look.

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

https://codereview.appspot.com/7064066/diff/2001/environs/agent/agent.go#newc...
environs/agent/agent.go:85: return fmt.Errorf("invalid server address %q", a)
On 2013/01/10 12:57:18, TheMue wrote:
> Not sure about this test. It only checks the format in a very simple way. But
> even if that is correct the server address may be invalid. So maybe the error
> returned when trying to open an invalid one is enough and more helpful.

this test is also invoked when writing the configuration, not just when reading,
so i think it's worth leaving in. FWIW, the test has just been moved from the
same thing in cmd/jujud, so we're not adding anything particularly new here.

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

https://codereview.appspot.com/7064066/diff/2001/environs/agent/agent_test.go...
environs/agent/agent_test.go:128: // move the configuration file to a different
directory
On 2013/01/10 12:57:18, TheMue wrote:
> "Move", just to be consistent. ;)

Done.

[fwereade, 2013-01-10 13:21:49]

LGTM -- json justified over yaml online on the basis that yaml doesn't deal with
[]byte well.

[rog, 2013-01-10 18:35:17]

*** Submitted:

environs/agent: new package

For the moment, it only deals with settings, but some
functionality will probably move from environs/tools.go
into this package in a future CL.

I decided to write all the settings to a single file rather than the
current multiple files) because it's less code, more easily extensible,
and if we do want different goroutines in an agent to write different
aspects of the file, it would be straightforward to add some locking
around the write.

R=TheMue, fwereade
CC=
https://codereview.appspot.com/7064066