accumulo icon indicating copy to clipboard operation
accumulo copied to clipboard

Update Accumulo utilities to extend KeywordExecutable

Open cshannon opened this issue 2 years ago • 6 comments

Must utility applications extend KeywordExecutable. However, there are some that don't and these should be updated to do so. These applications are meant to be used with the accumulo command.

I did a search and here is a list of some examples that could be updated as part of this issue. They can be checked off as they get merged to mark as completed.

I will create PRs for these and submit them.

cshannon avatar Jul 13 '22 17:07 cshannon

I submitted PRs for a couple of these so far, I will continue with more this weekend. Not sure if there are any existing tests that need to be updated besides the one keyword I test but I can do a search as well.

cshannon avatar Jul 13 '22 19:07 cshannon

@dlmarion and @EdColeman - Ok this is ready for review, I went ahead and tackled 8 utilities for now, each with their own PR to review in isolation. I verified everything works using Uno and the accumulo command for each utility.

For the UsageGroup all the commands are just added under the default OTHER but this can be changed easily. Also some of these may be a candidate to be encapsulated under the Admin command but for now I just made them their own commands.

cshannon avatar Jul 16 '22 15:07 cshannon

Where you able to test each of these utilities? Some may be in an unknown working condition. Adding the Keywords to the commands will make them show up under the main accumulo command, making them more visible to the user. I think this is good for some, DumpZookeeper and TableDiskUsage for example, are more general purpose utilities. Other commands such as ChangeSecret and the Delete and Restore ZK, might be better served under the Admin command since these should only be used on a rare occasion.

milleruntime avatar Jul 18 '22 14:07 milleruntime

@milleruntime -

  1. Testing - I ran all the commands against Uno and they seemed to work but since I'm pretty new I wasn't sure how to verify the output on all of them. I looked around and didn't really see any existing unit tests for the utilities themselves to verify they work or functionality. Maybe another task should be to actually write some unit tests for these commands to verify they work (using mini accumulo, etc). I could create a new issue and PRs to add tests (if you agree they should be a separate issue)
  2. I agree about moving some commands as I figured some of them would make more sense as part of the Admin command as I mentioned but wasn't sure which ones. I worked on 8 so far, I can update my PRs and move ChangeSecret, Delete and Restore ZK to the Admin command. Any others you think?
  3. I am also going to search around the code now and take a look now to see if there's anymore commands to fix but if you know of any others let me know.

cshannon avatar Jul 18 '22 14:07 cshannon

@milleruntime - I updated the PRs for DeleteZooInstance, RestoreZooInstance and ChangeSecret to now be included as part of the admin command.

cshannon avatar Jul 18 '22 17:07 cshannon

@milleruntime - I just went through and rebased all the PRs off the latest off main and fixed the descriptions and commit messages for them all.

cshannon avatar Jul 19 '22 13:07 cshannon