launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/juju-core/mpv-publicstorage-no-longer-optional into lp:~maas-maintainers/juju-core/maas-provider-skeleton.

Commit message:
Environ.PublicStorage() is no longer optional.  Since MAAS only has one kind, return the same as Storage().

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jtv/juju-core/mpv-publicstorage-no-longer-optional/+merge/152821

I have also posted to the juju-dev mailing list to ask whether this is a sane thing to do.  Depending on the answer, we may come back to this later.


Jeroen
-- 
https://code.launchpad.net/~jtv/juju-core/mpv-publicstorage-no-longer-optional/+merge/152821
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/juju-core/mpv-publicstorage-no-longer-optional into lp:~maas-maintainers/juju-core/maas-provider-skeleton.

=== modified file 'environs/maas/environ.go'
--- environs/maas/environ.go	2013-03-12 03:51:49 +0000
+++ environs/maas/environ.go	2013-03-12 05:23:20 +0000
@@ -312,14 +312,17 @@
 	return environ.instances(nil)
 }
 
+// Storage is defined by the Environ interface.
 func (env *maasEnviron) Storage() environs.Storage {
 	env.ecfgMutex.Lock()
 	defer env.ecfgMutex.Unlock()
 	return env.storageUnlocked
 }
 
-func (*maasEnviron) PublicStorage() environs.StorageReader {
-	return nil
+// PublicStorage is defined by the Environ interface.
+func (env *maasEnviron) PublicStorage() environs.StorageReader {
+	// MAAS does not have separate public and private storage.
+	return env.Storage()
 }
 
 func (environ *maasEnviron) Destroy([]environs.Instance) error {

=== modified file 'environs/maas/environ_test.go'
--- environs/maas/environ_test.go	2013-03-12 03:46:14 +0000
+++ environs/maas/environ_test.go	2013-03-12 05:23:20 +0000
@@ -162,9 +162,14 @@
 	c.Check(specificStorage.environUnlocked, Equals, env)
 }
 
-func (suite *EnvironSuite) TestPublicStorageIsNotImplemented(c *C) {
+func (suite *EnvironSuite) TestPublicStorageReturnsStorage(c *C) {
 	env := suite.makeEnviron()
-	c.Check(env.PublicStorage(), IsNil)
+	storage := env.PublicStorage()
+	c.Check(storage, NotNil)
+	// The Storage object is really a maasStorage.
+	specificStorage := storage.(*maasStorage)
+	// Its environment pointer refers back to its environment.
+	c.Check(specificStorage.environUnlocked, Equals, env)
 }
 
 func (suite *EnvironSuite) TestStartInstanceStartsInstance(c *C) {