-
Notifications
You must be signed in to change notification settings - Fork 11
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
base: li-dev/release-3.6.2-1-backup
Are you sure you want to change the base?
Add RestoreCommand as CliCommand #29
Conversation
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) { |
There was a problem hiding this comment.
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"); |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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?
a5f3e48
to
8b16e3e
Compare
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.