Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add a juju subcommand for backups. #97

Closed

Conversation

ericsnowcurrently
Copy link
Contributor

This will fit on top of the client API that Michael is writing.

}

type BackupCommand struct {
/* XXX Shuld this really be an env-specific command? */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. You want to backup the data for a specific environment. Almost everything that "juju" does is in the context of 1 environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I'm not sure what I was thinking. :)

@jameinel
Copy link
Member

I like the structuring of the client code quite a bit. I'd make "run" a bit more clear about what it actually is.
Also, I would expect that landing the "juju backup" command would be a good time for us to completely remove the cmd/plugins/juju-backup plugin entirely.
We'll also want to end up figuring out how to do API compatibility. I'm hopeful that my code will get reviewed today, and should land quickly this week. Thus likely before the rest of the backup code can land, so I'll be happy to work with you guys in figuring out how to do Backup APIs and versioned API calls.

@jameinel
Copy link
Member

I don't think this is ready to be landed as in, and I don't think you need further reviews. So I'm going to close this for now. You're welcome to open it up again if you want more feedback, or when you feel it is ready to land, etc.

@jameinel jameinel closed this Jun 17, 2014
@ericsnowcurrently
Copy link
Contributor Author

re: API compatibility, you're referring to the potential disparity in capability between client and server, right? I spoke with Michael and it makes sense for me now. We'll sort that out. Michael's point to me was that for backups (and restore) the versions should line up. In that case, I guess making use of your API versioning could would make sense. :)

@ericsnowcurrently
Copy link
Contributor Author

Keep in mind that this changeset is dependent on Michael's addition of client.Backup(). So I hadn't intended on merging it as-is. I just wanted to get feedback. Mission accomplished and thank you!

@ericsnowcurrently ericsnowcurrently changed the title Add a juju subcommand for backups. WIP: Add a juju subcommand for backups. Jun 18, 2014
@ericsnowcurrently
Copy link
Contributor Author

hmm, I've prepended "WIP:" to the summary but can't re-open the PR for some reason. It's probably going to be easier to just create a new pull request (and link to this one) than to try to debug github. :)

ericsnowcurrently pushed a commit to ericsnowcurrently/juju that referenced this pull request May 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants