juju: always connect to State.

juju: always connect to State.

All uses of juju.Conn other than Bootstrap and Destroy require a state
connection, so make State an exported field of Conn and connect to it
when creating the Conn.

The few places that call Bootstrap can easily call Environ.Bootstrap
instead.

Also add environs.NewFromName to make it easier to open an
environment, rename juju.NewConn to juju.NewConnFromName, and add
juju.NewConnFromEnviron to join the dots.

https://code.launchpad.net/~rogpeppe/juju-core/048-juju-conn-state-always/+merge/122529

(do not edit description out of merge proposal)

[rog, 2012-09-03 14:15:48]

Please take a look.

[rog, 2012-09-03 14:19:03]

Please take a look.

[TheMue, 2012-09-03 14:27:49]

LGTM, only one question.

https://codereview.appspot.com/6488077/diff/3001/juju/deploy.go
File juju/deploy.go (right):

https://codereview.appspot.com/6488077/diff/3001/juju/deploy.go#newcode19
juju/deploy.go:19: name = ch.URL().Name // TODO sch.Meta().Name ?
sch? Or do you mean ch?

[rog, 2012-09-03 14:30:26]

https://codereview.appspot.com/6488077/diff/3001/juju/deploy.go
File juju/deploy.go (right):

https://codereview.appspot.com/6488077/diff/3001/juju/deploy.go#newcode19
juju/deploy.go:19: name = ch.URL().Name // TODO sch.Meta().Name ?
On 2012/09/03 14:27:49, TheMue wrote:
> sch? Or do you mean ch?

ch is probably right, but i'll leave this for a future CL as it's an unrelated
change.

[niemeyer, 2012-09-03 16:04:51]

Looks fine, although I'd prefer to see NewConn going away.

LGTM

https://codereview.appspot.com/6488077/diff/3001/juju/conn.go
File juju/conn.go (right):

https://codereview.appspot.com/6488077/diff/3001/juju/conn.go#newcode51
juju/conn.go:51: func NewConn(environName string) (*Conn, error) {
Can we kill this one, and rename the one above to NewConn? We already have
several different ways to build an environment. It feels bad to reproduce the
same helpers again everywhere we need an environment.

https://codereview.appspot.com/6488077/diff/3001/juju/deploy.go
File juju/deploy.go (right):

https://codereview.appspot.com/6488077/diff/3001/juju/deploy.go#newcode19
juju/deploy.go:19: name = ch.URL().Name // TODO sch.Meta().Name ?
On 2012/09/03 14:30:26, rog wrote:
> On 2012/09/03 14:27:49, TheMue wrote:
> > sch? Or do you mean ch?
> 
> ch is probably right, but i'll leave this for a future CL as it's an unrelated
> change.

Frank mentioned a trivial typo on a TODO comment. It seems trivial and polite to
fix it.

[rog, 2012-09-03 16:23:06]

Please take a look.

https://codereview.appspot.com/6488077/diff/3001/juju/conn.go
File juju/conn.go (right):

https://codereview.appspot.com/6488077/diff/3001/juju/conn.go#newcode51
juju/conn.go:51: func NewConn(environName string) (*Conn, error) {
On 2012/09/03 16:04:51, niemeyer wrote:
> Can we kill this one, and rename the one above to NewConn? We already have
> several different ways to build an environment. It feels bad to reproduce the
> same helpers again everywhere we need an environment.

I'm not sure. NewConn is the only constructor currently used in the non-test
code, and it seems a nice convenient default entry point.

If anything, I'd remove NewConnFromAttrs, being much less frequently used.

https://codereview.appspot.com/6488077/diff/3001/juju/deploy.go
File juju/deploy.go (right):

https://codereview.appspot.com/6488077/diff/3001/juju/deploy.go#newcode19
juju/deploy.go:19: name = ch.URL().Name // TODO sch.Meta().Name ?
On 2012/09/03 16:04:51, niemeyer wrote:
> On 2012/09/03 14:30:26, rog wrote:
> > On 2012/09/03 14:27:49, TheMue wrote:
> > > sch? Or do you mean ch?
> > 
> > ch is probably right, but i'll leave this for a future CL as it's an
unrelated
> > change.
> 
> Frank mentioned a trivial typo on a TODO comment. It seems trivial and polite
to
> fix it.

Done.

[rog, 2012-09-03 16:30:26]

Please take a look.

[niemeyer, 2012-09-03 17:24:17]

https://codereview.appspot.com/6488077/diff/3001/juju/conn.go
File juju/conn.go (right):

https://codereview.appspot.com/6488077/diff/3001/juju/conn.go#newcode51
juju/conn.go:51: func NewConn(environName string) (*Conn, error) {
On 2012/09/03 16:23:06, rog wrote:
> On 2012/09/03 16:04:51, niemeyer wrote:
> > Can we kill this one, and rename the one above to NewConn? We already have
> > several different ways to build an environment. It feels bad to reproduce
the
> > same helpers again everywhere we need an environment.
> 
> I'm not sure. NewConn is the only constructor currently used in the non-test
> code, and it seems a nice convenient default entry point.
> 
> If anything, I'd remove NewConnFromAttrs, being much less frequently used.

Okay, so at least can we try to make things more similar here and in environs:

environs.New(config)
environs.NewFromName(name)

juju.NewConn(environ)
juju.NewConnFromName(name)

[niemeyer, 2012-09-03 17:31:48]

LGTM with these.

[rog, 2012-09-03 17:37:57]

*** Submitted:

juju: always connect to State.

All uses of juju.Conn other than Bootstrap and Destroy require a state
connection, so make State an exported field of Conn and connect to it
when creating the Conn.

The few places that call Bootstrap can easily call Environ.Bootstrap
instead.

Also add environs.NewFromName to make it easier to open an
environment, rename juju.NewConn to juju.NewConnFromName, and add
juju.NewConnFromEnviron to join the dots.

R=TheMue, niemeyer
CC=
https://codereview.appspot.com/6488077