Code review comment for lp://staging/~dimitern/juju-core/038-upgrade-charm-switch
Revision history for this message
| William Reade (fwereade) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
9199653
demo site
(Get the code!)
I'm on the fence a little; let's have a chat about how bad the
revision-bumping situation is, it might be that the benefits of your
implementation will win out.
https:/
File cmd/juju/
https:/
cmd/juju/
charm %q", curl)
On 2013/04/29 09:53:21, dimitern wrote:
> On 2013/04/28 10:41:25, fwereade wrote:
> > Hmm. If the --force flag is set to something different to before, we
might
> > actually want to allow this case (and other error below)?.
Pre-existing bug,
> > doesn't have to be addressed in this CL, but if it isn't it should
be
> recorded.
> I added a TODO
A bug would be great too please.
https:/
File cmd/juju/
https:/
cmd/juju/
An explicit revision can be chosen with the --revision flag.
https:/
cmd/juju/
configuration settings.
How about:
The --switch flag allows you to replace the charm with an entirely
different one. The new charm's URL and revision are inferred as they
would be when running a deploy command.
Please note that --switch is dangerous, because juju only has limited
information with which to determine compatibility; the operation will
succeed, regardless of potential havoc, so long as the following
conditions hold:
- The new charm must declare all relations that the service is currently
participating in.
- All config settings shared by the old and new charms must have the
same types.
--switch and --revision are mutually exclusive.
https:/
cmd/juju/
charm.
Drop this para, --revision is best described at the top, I think; we
just need to mention it won't work with switch.
https:/
cmd/juju/
"charm URL to upgrade to")
"crossgrade to a different charm" ?
https:/
cmd/juju/
"revision number to upgrade to")
"explicit revision of current charm" ?
https:/
cmd/juju/
On 2013/04/29 09:53:21, dimitern wrote:
> On 2013/04/28 10:41:25, fwereade wrote:
> > I think this can all be simpler.
> >
> > oldURL, _ := service.CharmURL()
> > var newURL *charm.URL
> > if c.SwitchURL != "" {
> > // A new charm URL was explicitly specified.
> > conf, err := conn.State.
> > if err != nil {
> > return err
> > }
> > newURL, err = charm.InferURL(
> > if err != nil {
> > return err
> > }
> > } else {
> > // No new URL was specified, but a revision might have been.
> > newURL = oldURL.
> > }
> > repo, err := charm.InferRepo
> > if err != nil {
> > return err
> > }
> > // If no revision was explicitly specified by either Revision or
> > // SwitchURL, discover the latest from the repo.
> > if newURL.Revision == -1 {
> > latest, err := repo.Latest(newURL)
> > if err != nil {
> > return err
> > }
> > newURL = newURL.
> > }
> > if *newURL == *oldURL {
> > if _, isLocal := repo.(*
> > // etc
> I prefer to keep it as is, if you don't mind, and I don't think it's
any more
> clear the suggested way.
I find the current way rather unclear; it's a little tricky to keep
track of curl, scurl, bumpRevision, explicitRevision, rev, and
considerBumpRev
have revisions embedded as well. And I'm not sure that bumpRevision is
even used any more...
If my suggestion is unclear, perhaps I should have commented it better;
but I think it's definitely easier to understand and keep track of
oldURL and newURL than all that other stuff... isn't it?
I'm not sure mine has ideal behaviour when --force and --revision
interact with local repos, but I don't think we have the *capacity* to
do the Right Thing there (ie upload a charm with a unique revision: but
at the moment we're screwed here in 2 or 3 different ways anyway, so it
seems to be a moot point).
> Does it fulfill the required set of cases? I think so.
I'm not 100% sure, and that's the problem.
https:/