Skip to content

consolidate InfoViewer and PropEditor #6458

Open
ArbaazKhan1 wants to merge 1 commit into
apache:mainfrom
ArbaazKhan1:accumulo-6428
Open

consolidate InfoViewer and PropEditor #6458
ArbaazKhan1 wants to merge 1 commit into
apache:mainfrom
ArbaazKhan1:accumulo-6428

Conversation

@ArbaazKhan1

Copy link
Copy Markdown
Contributor

Closes issue #6428

Created a new zk props command to consolidate ZooKeeper property inspection and editing. Both legacy commands are depreciated but remain and functional for backwards compatibility. The new zk props core sub-commands operate as such:

  • Default / scope flags (-t, -ns, -r) --> reproduces ZooPropEditor single-scope print output
  • --set, set a property for the resolved scope
  • --delete, delete a property for the resolved scope
  • --get, reproduces ZooInfoViewer --print-props multi-scope report, using --system, --namespaces, and --tables filter flags

Comment on lines +200 to +205
if ((!systemOpt || !namespacesOpt.isEmpty() || !tablesOpt.isEmpty()) && !getOpt
&& setOpt.isEmpty() && deleteOpt.isEmpty()) {
if (!namespacesOpt.isEmpty() || !tablesOpt.isEmpty()) {
throw new IllegalArgumentException("--namespaces and --tables are only valid with --get");
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I'm understanding things correctly here, I think we should be rejecting --tables and --namespaces for --set and --delete as opposed to whats happening now: only when --get is absent.

Suggested change
if ((!systemOpt || !namespacesOpt.isEmpty() || !tablesOpt.isEmpty()) && !getOpt
&& setOpt.isEmpty() && deleteOpt.isEmpty()) {
if (!namespacesOpt.isEmpty() || !tablesOpt.isEmpty()) {
throw new IllegalArgumentException("--namespaces and --tables are only valid with --get");
}
}
boolean isMutating = !setOpt.isEmpty() || !deleteOpt.isEmpty();
if (!namespacesOpt.isEmpty() || !tablesOpt.isEmpty()) {
if (isMutating) {
throw new IllegalArgumentException(
"--namespaces and --tables are not valid with --set or --delete");
}
if (!getOpt) {
throw new IllegalArgumentException("--namespaces and --tables are only valid with --get");
}
}

Comment on lines +137 to +138
v.tablesOpt.addAll(opts.tablesOpt);
v.namespacesOpt.addAll(opts.namespacesOpt);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

--get advertises the ability to accept --table and --namespace as well but only the plural tables and namespaces opts are copied.

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.

Consolidate prop-editor and info-viewer into a unified ZooKeeper props sub-command

2 participants