Merge lp:~cmars/juju-core/local-repo-errmsg into lp:~go-bot/juju-core/trunk

[Casey Marshall]

Error msg for local charm deploy missing series.

Add error message when unable to resolve series for local repository.
Update juju deploy doc message to describe behavior.

https://codereview.appspot.com/86160043/

Error msg for local charm deploy missing series.

Add error message when unable to resolve series for local repository.
Update juju deploy doc message to describe behavior.

https://codereview.appspot.com/86160043/

[Casey Marshall (cmars)]

Reviewers: mp+215021_code.launchpad.net,

Message:
Please take a look.

Description:
Error msg for local charm deploy missing series.

Add error message when unable to resolve series for local repository.
Update juju deploy doc message to describe behavior.

https://code.launchpad.net/~cmars/juju-core/local-repo-errmsg/+merge/215021

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/86160043/

Affected files (+14, -4 lines):
   A [revision details]
   M cmd/juju/common.go
   M cmd/juju/deploy.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140409180231-hnxao50offx5c4c9
+New revision: <email address hidden>

Index: cmd/juju/common.go
=== modified file 'cmd/juju/common.go'
--- cmd/juju/common.go 2014-04-07 10:05:04 +0000
+++ cmd/juju/common.go 2014-04-09 19:31:16 +0000
@@ -4,6 +4,8 @@
  package main

  import (
+ "fmt"
+
   "launchpad.net/juju-core/charm"
   "launchpad.net/juju-core/cmd"
   "launchpad.net/juju-core/environs"
@@ -65,6 +67,12 @@
   }
   // Otherwise, look up the best supported series for this charm
   if series == "" {
+ if ref.Schema == "local" {
+ possibleUrl := &charm.URL{Reference: ref, Series: "precise"}
+ logger.Errorf(`The series is not specified in the environment
(default-series) or with the charm. Did you mean:
+ %s`, possibleUrl.String())
+ return nil, fmt.Errorf("cannot resolve series for charm: %q", ref)
+ }
    return client.ResolveCharm(ref)
   }
   return &charm.URL{Reference: ref, Series: series}, nil

Index: cmd/juju/deploy.go
=== modified file 'cmd/juju/deploy.go'
--- cmd/juju/deploy.go 2014-04-09 06:35:07 +0000
+++ cmd/juju/deploy.go 2014-04-09 19:31:16 +0000
@@ -49,12 +49,12 @@
  For cs:~user/precise/mysql
    cs:~user/mysql

-For local:precise/mysql
- local:mysql
-
-In all cases, a versioned charm URL will be expanded as expected (for
example,
+In these cases, a versioned charm URL will be expanded as expected (for
example,
  mysql-33 becomes cs:precise/mysql-33).

+However, for local charms, such as local:precise/mysql, one must specify
the
+series.
+
  <service name>, if omitted, will be derived from <charm name>.

  Constraints can be specified when using deploy by specifying the
--constraints

[Curtis Hovey (sinzui)]

Hi Casey.

I am not a reviewer, just an interested party.

https://codereview.appspot.com/86160043/diff/1/cmd/juju/deploy.go
File cmd/juju/deploy.go (right):

https://codereview.appspot.com/86160043/diff/1/cmd/juju/deploy.go#newcode55
cmd/juju/deploy.go:55: However, for local charms, such as
local:precise/mysql, one must specify the
I see "assuming a current default series of" in the text above. That is
ambiguous. I din't know if that means the charm's default series or the
environment's default-series.

Maybe this phrasing is helpful
     However, for local charms, where the default-series is not specified
in the environment, one must specify the series.

Older versions of juju had default-series in the env template created by
"juju init". Many of my envs have a define default-series because juju
put it there. So new local users of the past few months are the common
victims of this change. Maybe the local template should a default-series
line commented out to help users discover the feature.

https://codereview.appspot.com/86160043/

[Casey Marshall (cmars)]

Please take a look.

https://codereview.appspot.com/86160043/diff/1/cmd/juju/deploy.go
File cmd/juju/deploy.go (right):

https://codereview.appspot.com/86160043/diff/1/cmd/juju/deploy.go#newcode55
cmd/juju/deploy.go:55: However, for local charms, such as
local:precise/mysql, one must specify the
On 2014/04/09 20:13:33, curtis wrote:
> I see "assuming a current default series of" in the text above. That
is
> ambiguous. I din't know if that means the charm's default series or
the
> environment's default-series.

Added a sentence to disambiguate.

> Maybe this phrasing is helpful
> However, for local charms, where the default-series is not
specified in the
> environment, one must specify the series.

Done.

> Older versions of juju had default-series in the env template created
by "juju
> init". Many of my envs have a define default-series because juju put
it there.
> So new local users of the past few months are the common victims of
this change.
> Maybe the local template should a default-series line commented out to
help
> users discover the feature.

Where does this template live? I see the templates for specific
providers, but none of them contain a commented default-series: as far
as I can tell.

My environment.yaml has a commented default-series, but it was generated
ages ago.

https://codereview.appspot.com/86160043/

[Casey Marshall (cmars)]

Updated 'juju deploy' doc, what do you think?

https://codereview.appspot.com/86160043/

[Casey Marshall (cmars)]

Please take a look.

https://codereview.appspot.com/86160043/

[Tim Penhey (thumper)]

LGTM

perhaps an example in the help doc?

https://codereview.appspot.com/86160043/diff/40001/cmd/juju/deploy.go
File cmd/juju/deploy.go (right):

https://codereview.appspot.com/86160043/diff/40001/cmd/juju/deploy.go#newcode58
cmd/juju/deploy.go:58: environment, one must specify the series.
Worthwhile adding an example here?

https://codereview.appspot.com/86160043/

=== modified file 'cmd/juju/common.go'
--- cmd/juju/common.go	2014-04-09 16:36:12 +0000
+++ cmd/juju/common.go	2014-04-09 22:31:41 +0000
@@ -4,6 +4,8 @@
 package main
 
 import (
+	"fmt"
+
 	"launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/cmd"
 	"launchpad.net/juju-core/environs"
@@ -65,6 +67,12 @@
 	}
 	// Otherwise, look up the best supported series for this charm
 	if series == "" {
+		if ref.Schema == "local" {
+			possibleUrl := &charm.URL{Reference: ref, Series: "precise"}
+			logger.Errorf(`The series is not specified in the environment (default-series) or with the charm. Did you mean:
+	%s`, possibleUrl.String())
+			return nil, fmt.Errorf("cannot resolve series for charm: %q", ref)
+		}
 		return client.ResolveCharm(ref)
 	}
 	return &charm.URL{Reference: ref, Series: series}, nil
 
=== modified file 'cmd/juju/deploy.go'
--- cmd/juju/deploy.go	2014-04-09 16:36:12 +0000
+++ cmd/juju/deploy.go	2014-04-09 22:31:41 +0000
@@ -39,8 +39,7 @@
 
 const deployDoc = `
 <charm name> can be a charm URL, or an unambiguously condensed form of it;
-assuming a current default series of "precise", the following forms will be
-accepted.
+assuming a current series of "precise", the following forms will be accepted:
 
 For cs:precise/mysql
   mysql
@@ -49,12 +48,16 @@
 For cs:~user/precise/mysql
   cs:~user/mysql
 
-For local:precise/mysql
-  local:mysql
+The current series is determined first by the default-series environment
+setting, followed by the preferred series for the charm in the charm store.
 
-In all cases, a versioned charm URL will be expanded as expected (for example,
+In these cases, a versioned charm URL will be expanded as expected (for example,
 mysql-33 becomes cs:precise/mysql-33).
 
+However, for local charms, when the default-series is not specified in the
+environment, one must specify the series. For example:
+  local:precise/mysql
+
 <service name>, if omitted, will be derived from <charm name>.
 
 Constraints can be specified when using deploy by specifying the --constraints
 
=== modified file 'provider/local/environprovider.go'
--- provider/local/environprovider.go	2014-04-02 11:35:49 +0000
+++ provider/local/environprovider.go	2014-04-09 22:31:41 +0000
@@ -262,6 +262,10 @@
     #
     # network-bridge: lxcbr0
 
+    # The default series to deploy the state-server and charms on.
+    #
+    # default-series: precise
+
 `[1:]
 }