Code review comment for lp:~dimitern/juju-core/394-network-name-to-id-and-add-tags

At least one of us is confused... possibly both. Hopefully these
comments will elucidate where I'm coming from; ping me for a chat if you
need to.

https://codereview.appspot.com/86010044/diff/20001/names/network_test.go
File names/network_test.go (right):

https://codereview.appspot.com/86010044/diff/20001/names/network_test.go#newcode23
names/network_test.go:23: c.Assert(names.IsNetwork("42"), jc.IsTrue)
Let's not let users do this. Same restrictions as service names perhaps?

https://codereview.appspot.com/86010044/diff/20001/names/tag_test.go
File names/tag_test.go (left):

https://codereview.appspot.com/86010044/diff/20001/names/tag_test.go#oldcode130
names/tag_test.go:130: // TODO(rog) environment and user, when they have
Tag functions.
Thanks

https://codereview.appspot.com/86010044/diff/20001/state/api/params/internal.go
File state/api/params/internal.go (right):

https://codereview.appspot.com/86010044/diff/20001/state/api/params/internal.go#newcode316
state/api/params/internal.go:316: Tag string
Hmm, doesn't this need both the tag and the id?

https://codereview.appspot.com/86010044/diff/20001/state/apiserver/client/client.go
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/86010044/diff/20001/state/apiserver/client/client.go#newcode295
state/apiserver/client/client.go:295: includeNetworks, err :=
networkTagsToIds(args.IncludeNetworks)
tags to names, not ids, surely?

I've been thinking of names as juju vocab, ids as provider vocab, and
tags as derived from names. Am I confused?

https://codereview.appspot.com/86010044/diff/20001/state/apiserver/provisioner/provisioner.go
File state/apiserver/provisioner/provisioner.go (right):

https://codereview.appspot.com/86010044/diff/20001/state/apiserver/provisioner/provisioner.go#newcode464
state/apiserver/provisioner/provisioner.go:464: // Convert network ids
to tags before returning.
Doesn't this need to be ids? They're for consumption by the provider,
right? Or will the provisioner be using the tags to look up the networks
and get the ids to pass on to StartInstance?

https://codereview.appspot.com/86010044/diff/20001/state/networkinterfaces.go
File state/networkinterfaces.go (right):

https://codereview.appspot.com/86010044/diff/20001/state/networkinterfaces.go#newcode23
state/networkinterfaces.go:23: NetworkId string
This is an internal reference to a juju entity. Shouldn't it be using
the name?

https://codereview.appspot.com/86010044/diff/20001/state/networks.go
File state/networks.go (right):

https://codereview.appspot.com/86010044/diff/20001/state/networks.go#newcode39
state/networks.go:39: }
It might make things clearer if we had an explicit Name() func here that
just happened to return the provider id?

Would be clearest of all tbh of we had separate Name and ProviderId
fields...

https://codereview.appspot.com/86010044/diff/20001/state/state.go
File state/state.go (right):

https://codereview.appspot.com/86010044/diff/20001/state/state.go#newcode1021
state/state.go:1021: // TODO(dimitern) Start using sequences for id,
like for machines. For
huh? surely names need to be user-specified?

https://codereview.appspot.com/86010044/diff/20001/state/state.go#newcode1062
state/state.go:1062: func (st *State) Network(id string) (*Network,
error) {
I don't think we ever want to identify a network by provider id, do we?
state talks juju language -- we don't ever look up a machine by instance
id, do we?

https://codereview.appspot.com/86010044/