-
Notifications
You must be signed in to change notification settings - Fork 588
HDDS-4046. Extensible subcommands for CLI applications #1276
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
Conversation
adoroszlai
left a comment
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.
Thanks @elek for this improvement.
| GetServiceRolesSubcommand.class | ||
| }) | ||
| public class OMAdmin extends GenericCli { | ||
| public class OMAdmin extends GenericCli implements SubcommandWithParent { |
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.
Won't we need
@MetaInfServices(SubcommandWithParent.class)
too?
Currently OMAdmin is added to OzoneAdmin from the parent, so it's not a problem. (#1285 will allow inverting subcommand registration for OzoneAdmin, too.)
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.
Yes, it's missing. 🦅 👁️ 🏆
Also, I can remove the OmAdmin from the annotations of OzoneAdmin. (Fortunately annotations and subcommands added programmatically can work together).
|
Thanks @elek for updating the patch. Tested that |
* master: (28 commits) HDDS-4037. Incorrect container numberOfKeys and usedBytes in SCM after key deletion (apache#1295) HDDS-3232. Include the byteman scripts in the distribution tar file (apache#1309) HDDS-4095. Byteman script to debug HCFS performance (apache#1311) HDDS-4057. Failed acceptance test missing from bundle (apache#1283) HDDS-4040. [OFS] BasicRootedOzoneFileSystem to support batchDelete (apache#1286) HDDS-4061. Pending delete blocks are not always included in #BLOCKCOUNT metadata (apache#1288) HDDS-4067. Implement toString for OMTransactionInfo (apache#1300) HDDS-3878. Make OMHA serviceID optional if one (but only one) is defined in the config (apache#1149) HDDS-3833. Use Pipeline choose policy to choose pipeline from exist pipeline list (apache#1096) HDDS-3979. Make bufferSize configurable for stream copy (apache#1212) HDDS-4048. Show more information while SCM version info mismatch (apache#1278) HDDS-4078. Use HDDS InterfaceAudience/Stability annotations (apache#1302) HDDS-4034. Add Unit Test for HadoopNestedDirGenerator. (apache#1266) HDDS-4076. Translate CSI.md into Chinese (apache#1299) HDDS-4046. Extensible subcommands for CLI applications (apache#1276) HDDS-4051. Remove whitelist/blacklist terminology from Ozone (apache#1306) HDDS-4055. Cleanup GitHub workflow (apache#1282) HDDS-4042. Update documentation for the GA release (apache#1269) HDDS-4066. Add core-site.xml to intellij configuration (apache#1292) HDDS-4073. Remove leftover robot.robot (apache#1297) ...
What changes were proposed in this pull request?
HDDS-3814 proposed a new subcommand which deletes the column families from the rocksdb tables.
But during the discussion there was no consensus if it's safe to add or not.
This patch makes the sub-commands easy to extend. Anybody can extend the main ozone shell commands with new sub-commands.
Sub-commands can be added to the classpath and activated by Service Provider Interface (meta-inf/services/...).
This is an optional feature, current approach works forward (annotation based sub-command definition).
And it's not only about HDDS-3814.
It makes it easier to organize the sub-commands. (For example RDBStore related commands can be moved to the rdb classes, they will be picked up).
It makes possible to provide additional, ad-hoc tools during debug or support which can help to solve/debug specific problems.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4046
How was this patch tested?
With CI. The changed subcommands are covered by acceptance tests. Also tested manually with
ozone sh.