Fail when run against Python juju environment

environs/open.go

 // Copyright 2011, 2012, 2013 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package environs
 
 import (
         "fmt"
+        "io/ioutil"
         "launchpad.net/juju-core/environs/config"
+        "launchpad.net/juju-core/errors"
 )
 
 // Open creates a new Environ using the environment configuration with the
 // given name. If name is empty, the default environment will be used.
 func (envs *Environs) Open(name string) (Environ, error) {
         if name == "" {
                 name = envs.Default
                 if name == "" {
                         return nil, fmt.Errorf("no default environment found")
                 }
@@ skipping 30 matching lines @@
 }
 
 // New returns a new environment based on the provided configuration.
 func New(config *config.Config) (Environ, error) {
         p, err := Provider(config.Type())
         if err != nil {
                 return nil, err
         }
         return p.Open(config)
 }
+
+// Checks if an environment has a bootstrap-verify that is written by

[dimitern, 2013/06/28 16:34:08]

// CheckEnvironment checks ...

[Danilo, 2013/06/29 03:03:11]

Fixed.

+// juju-core commands (as compared to one being written by Python juju).
+//
+// If there is no bootstrap-verify file in the storage, it is still
+// considered to be a Juju-core environment since early versions have
+// not written it out.
+//
+// Returns InvalidEnvironmentError on failure, nil otherwise.
+func CheckEnvironment(environ Environ) error {
+        storage := environ.Storage()
+        reader, err := storage.Get(verificationFilename)
+        if errors.IsNotFoundError(err) {
+                // When verification file does not exist, this is a juju-core
+                // environment.
+                return nil
+        } else if err == nil {
+                content, err := ioutil.ReadAll(reader)
+                if err == nil && string(content) == verificationContent {
+                        // Content matches what juju-core puts in the
+                        // verificationFilename.
+                        return nil
+                }
+        }
+        return InvalidEnvironmentError

[fwereade, 2013/06/28 15:51:19]

I'd prefer to distinguish between "couldn't read file" and "bad content", if
possible. eg

if reader, err := storage.Get(verificationFilename); errors.IsNotFoundError(err)
{
    return nil
} else if err != nil {
    return err
} else if content, err := ioutil.ReadAll(reader); err != nil {
    return err
} else if content != verificationContent {
    return InvalidEnvironmentError
}
return nil

...although some would prefer that the ladder be split up a bit and I'd be fine
with that too.

[Danilo, 2013/06/29 03:03:11]

Done it mostly like this (though I very much dislike "if x := expression; x"
syntax for readability purposes).

Also, the reasoning I was not propagating the error from internal subcalls is
your comment on https://codereview.appspot.com/9876043/#msg13 — I understood it
to mean a better pattern is to simply log any internal errors we hit (which,
granted, I did not do here), and return a single error message from the
function. This is something that'd hugely benefit from a coding style guide
recommendation, to avoid any unnecessary back-and-forth.

+}