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

Add RestoreCommand as CliCommand #29

Draft
wants to merge 1 commit into
base: li-dev/release-3.6.2-1-backup
Choose a base branch
from

Conversation

narendly
Copy link

@narendly narendly commented Feb 9, 2021

This commit adds restore as one of the ZkCli commands. We want to support this in the command line client so that it will be easy to use for ZK operators along with other CLI commands. Note that this commit only adds the skeleton and the main restoration logic will have to be implemented in a future commit.

This commit adds restore as one of the ZkCli commands. We want to support this in the command line client so that it will be easy to use for ZK operators along with other CLI commands. Note that this commit only adds the skeleton and the main restoration logic will have to be implemented in a future commit.
*/
public void parseArgs(String[] args) throws ConfigException {
// Check the num of args
if (args.length != 4 && args.length != 5) {

Choose a reason for hiding this comment

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

From a cli perspective, it is not a good idea to fix the number of arguments. Also order of arguments should not matter. The better way could be scanning the input arguments to find whether there is a matching option.

try {
zxidToRestore = Long.parseLong(numStr, base);
} catch (NumberFormatException nfe) {
System.err.println("Invalid number specified for restore point");

Choose a reason for hiding this comment

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

NIT: you may output the numStr as well.

// TODO: Construct a BackupConfig
BackupConfig backupConfig = new BackupConfig.Builder().build().get();

storage = new FileSystemBackupStorage(backupConfig);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use reflection to make this flexible too?

@narendly narendly force-pushed the li-dev/release-3.6.2-1-backup branch from a5f3e48 to 8b16e3e Compare April 23, 2021 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants