You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/08/14 20:50:13 UTC
[GitHub] [accumulo] cshannon opened a new pull request, #2881: Cleanup Accumulo Admin utilities
cshannon opened a new pull request, #2881:
URL: https://github.com/apache/accumulo/pull/2881
Remove main methods from utilities now that they are exposed through
KeywordExecutable and the accumulo command. Also sort the commands in
the help section alphabetically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime merged pull request #2881: Cleanup Accumulo Admin utilities
Posted by GitBox <gi...@apache.org>.
milleruntime merged PR #2881:
URL: https://github.com/apache/accumulo/pull/2881
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on pull request #2881: Cleanup Accumulo Admin utilities
Posted by GitBox <gi...@apache.org>.
cshannon commented on PR #2881:
URL: https://github.com/apache/accumulo/pull/2881#issuecomment-1214447037
@milleruntime - This is ready for review and removes main methods and sorts the commands in help alphabetically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on a diff in pull request #2881: Cleanup Accumulo Admin utilities
Posted by GitBox <gi...@apache.org>.
cshannon commented on code in PR #2881:
URL: https://github.com/apache/accumulo/pull/2881#discussion_r950304119
##########
server/base/src/main/java/org/apache/accumulo/server/util/Admin.java:
##########
@@ -226,49 +226,52 @@ public void execute(final String[] args) {
JCommander cl = new JCommander(opts);
cl.setProgramName("accumulo admin");
- CheckTabletsCommand checkTabletsCommand = new CheckTabletsCommand();
- cl.addCommand("checkTablets", checkTabletsCommand);
-
ChangeSecretCommand changeSecretCommand = new ChangeSecretCommand();
cl.addCommand("changeSecret", changeSecretCommand);
+ CheckTabletsCommand checkTabletsCommand = new CheckTabletsCommand();
+ cl.addCommand("checkTablets", checkTabletsCommand);
+
DeleteZooInstanceCommand deleteZooInstanceOpts = new DeleteZooInstanceCommand();
cl.addCommand("deleteZooInstance", deleteZooInstanceOpts);
- RestoreZooCommand restoreZooOpts = new RestoreZooCommand();
- cl.addCommand("restoreZoo", restoreZooOpts);
+ DumpConfigCommand dumpConfigCommand = new DumpConfigCommand();
+ cl.addCommand("dumpConfig", dumpConfigCommand);
ListInstancesCommand listIntancesOpts = new ListInstancesCommand();
cl.addCommand("listInstances", listIntancesOpts);
Review Comment:
@DomGarguilo - good catch, typo fixed. Is this good to merge or do you want to wait for @milleruntime to take a look too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on pull request #2881: Cleanup Accumulo Admin utilities
Posted by GitBox <gi...@apache.org>.
cshannon commented on PR #2881:
URL: https://github.com/apache/accumulo/pull/2881#issuecomment-1220530326
KeywordStartIT has been fixed. The issue was I inadvertently removed the main method from the DumpZookeeper command which is still marked with KeywordExecutable and not part of the Admin command.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a diff in pull request #2881: Cleanup Accumulo Admin utilities
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2881:
URL: https://github.com/apache/accumulo/pull/2881#discussion_r950255806
##########
server/base/src/main/java/org/apache/accumulo/server/util/Admin.java:
##########
@@ -226,49 +226,52 @@ public void execute(final String[] args) {
JCommander cl = new JCommander(opts);
cl.setProgramName("accumulo admin");
- CheckTabletsCommand checkTabletsCommand = new CheckTabletsCommand();
- cl.addCommand("checkTablets", checkTabletsCommand);
-
ChangeSecretCommand changeSecretCommand = new ChangeSecretCommand();
cl.addCommand("changeSecret", changeSecretCommand);
+ CheckTabletsCommand checkTabletsCommand = new CheckTabletsCommand();
+ cl.addCommand("checkTablets", checkTabletsCommand);
+
DeleteZooInstanceCommand deleteZooInstanceOpts = new DeleteZooInstanceCommand();
cl.addCommand("deleteZooInstance", deleteZooInstanceOpts);
- RestoreZooCommand restoreZooOpts = new RestoreZooCommand();
- cl.addCommand("restoreZoo", restoreZooOpts);
+ DumpConfigCommand dumpConfigCommand = new DumpConfigCommand();
+ cl.addCommand("dumpConfig", dumpConfigCommand);
ListInstancesCommand listIntancesOpts = new ListInstancesCommand();
cl.addCommand("listInstances", listIntancesOpts);
Review Comment:
```suggestion
ListInstancesCommand listInstancesOpts = new ListInstancesCommand();
cl.addCommand("listInstances", listInstancesOpts);
```
I know you didn't make this change but thought it would be worth it to fix this typo while changes are being made to this file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on pull request #2881: Cleanup Accumulo Admin utilities
Posted by GitBox <gi...@apache.org>.
cshannon commented on PR #2881:
URL: https://github.com/apache/accumulo/pull/2881#issuecomment-1215061234
> I think you may need to revise `KeywordStartIT.checkHasMain`. I ran it on this branch and it fails since some of the main methods were removed and that test expects them to be there.
Ah yeah, thanks I forgot about that. I'll update it when I get a chance and run through the Sunny test profile to verify.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org