environs/configstore: Write methods

environs/configstore: Write methods

This enables the environment endpoint info to be written.

https://code.launchpad.net/~rogpeppe/juju-core/397-configstore-write/+merge/186471

Requires: https://code.launchpad.net/~rogpeppe/juju-core/396-newclientfromname/+merge/186303

(do not edit description out of merge proposal)

[rog, 2013-09-19 06:41:13]

Please take a look.

[dimitern, 2013-09-19 11:59:42]

LGTM with a few comments below

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.go
File environs/configstore/disk.go (right):

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:75: return nil, fmt.Errorf("environment in progress
XXX better error message needed")
How about "environment configuration is not saved yet" (or not written yet) ?

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:101: func (info *environInfo)
SetAPIEndpoint(endpoint environs.APIEndpoint) {
doc comment

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:106: func (info *environInfo)
SetAPICredentials(creds environs.APICredentials) {
ditto

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:111: func (info *environInfo) Write() error {
ditto

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:135: func (info *environInfo) Destroy() error {
ditto

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:135: func (info *environInfo) Destroy() error {
ditto

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk_t...
File environs/configstore/disk_test.go (right):

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk_t...
environs/configstore/disk_test.go:101: // Also tests SetAPIEndpoint and
SetAPICredentials.
Then name it something like TestWriteAPIEndpointAPICredentials ?

[rog, 2013-09-19 14:34:20]

Please take a look.

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.go
File environs/configstore/disk.go (right):

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:75: return nil, fmt.Errorf("environment in progress
XXX better error message needed")
On 2013/09/19 11:59:42, dimitern wrote:
> How about "environment configuration is not saved yet" (or not written yet) ?

done slightly differently

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:101: func (info *environInfo)
SetAPIEndpoint(endpoint environs.APIEndpoint) {
On 2013/09/19 11:59:42, dimitern wrote:
> doc comment

Done.

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:106: func (info *environInfo)
SetAPICredentials(creds environs.APICredentials) {
On 2013/09/19 11:59:42, dimitern wrote:
> ditto

Done.

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:111: func (info *environInfo) Write() error {
On 2013/09/19 11:59:42, dimitern wrote:
> ditto

Done.

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk.g...
environs/configstore/disk.go:135: func (info *environInfo) Destroy() error {
On 2013/09/19 11:59:42, dimitern wrote:
> ditto

Done.

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk_t...
File environs/configstore/disk_test.go (right):

https://codereview.appspot.com/13778043/diff/3001/environs/configstore/disk_t...
environs/configstore/disk_test.go:101: // Also tests SetAPIEndpoint and
SetAPICredentials.
On 2013/09/19 11:59:42, dimitern wrote:
> Then name it something like TestWriteAPIEndpointAPICredentials ?

I made separate tests.

[fwereade, 2013-09-19 15:27:07]

LGTM, thanks

https://codereview.appspot.com/13778043/diff/8001/environs/configstore/disk.go
File environs/configstore/disk.go (left):

https://codereview.appspot.com/13778043/diff/8001/environs/configstore/disk.g...
environs/configstore/disk.go:47: path := d.envPath(envName)
Would you make sure this is using plain environment names without .yaml please?

https://codereview.appspot.com/13778043/diff/8001/environs/configstore/disk.go
File environs/configstore/disk.go (right):

https://codereview.appspot.com/13778043/diff/8001/environs/configstore/disk.g...
environs/configstore/disk.go:122: tmpFile, err := ioutil.TempFile(parent, "")
Hmm. Can we make this a name that can't be mistaken for an environment please?

[rog, 2013-09-19 16:00:54]

As discussed online, I'll leave the .yaml extension for the time being, pending
the discussion for the time we actually hook this code up.

https://codereview.appspot.com/13778043/diff/8001/environs/configstore/disk.go
File environs/configstore/disk.go (left):

https://codereview.appspot.com/13778043/diff/8001/environs/configstore/disk.g...
environs/configstore/disk.go:47: path := d.envPath(envName)
On 2013/09/19 15:27:07, fwereade wrote:
> Would you make sure this is using plain environment names without .yaml
please?

I'd prefer to have *some* suffix. To be discussed.

https://codereview.appspot.com/13778043/diff/8001/environs/configstore/disk.go
File environs/configstore/disk.go (right):

https://codereview.appspot.com/13778043/diff/8001/environs/configstore/disk.g...
environs/configstore/disk.go:122: tmpFile, err := ioutil.TempFile(parent, "")
On 2013/09/19 15:27:07, fwereade wrote:
> Hmm. Can we make this a name that can't be mistaken for an environment please?

I'm not sure that's possible if we use bare environment names
as filenames, as we don't currently impose any restrictions on
environment names.

[rog, 2013-09-19 16:37:48]

Please take a look.