-
Notifications
You must be signed in to change notification settings - Fork 477
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
HDDS-4056. Convert OzoneAdmin to pluggable model #1285
Conversation
public class ScmOption { | ||
|
||
@CommandLine.Spec(MIXEE) | ||
private CommandLine.Model.CommandSpec spec; |
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.
Today I learned it. Nice. Thanks.
Personally I feel it more natural. Moving some parameters before the last subcommand is strange syntax for me. (= I think the new one is easier to use) |
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.
+1 LGTM
Good to have this cleanup (and the tests)
And sorry for the late answer, as I remember I started to check it earlier but somehow forget to finish it.
Thanks @elek for reviewing and committing it. |
* master: (26 commits) HDDS-4167. Acceptance test logs missing if fails during cluster startup (apache#1366) HDDS-4121. Implement OmMetadataMangerImpl#getExpiredOpenKeys. (apache#1351) HDDS-3867. Extend the chunkinfo tool to display information from all nodes in the pipeline. (apache#1154) HDDS-4077. Incomplete OzoneFileSystem statistics (apache#1329) HDDS-3903. OzoneRpcClient support batch rename keys. (apache#1150) HDDS-4151. Skip the inputstream while offset larger than zero in s3g (apache#1354) HDDS-4147. Add OFS to FileSystem META-INF (apache#1352) HDDS-4137. Turn on the verbose mode of safe mode check on testlib (apache#1343) HDDS-4146. Show the ScmId and ClusterId in the scm web ui. (apache#1350) HDDS-4145. Bump version to 1.1.0-SNAPSHOT on master (apache#1349) HDDS-4109. Tests in TestOzoneFileSystem should use the existing MiniOzoneCluster (apache#1316) HDDS-4149. Implement OzoneFileStatus#toString (apache#1356) HDDS-4153. Increase default timeout in kubernetes tests (apache#1357) HDDS-2411. add a datanode chunk validator fo datanode chunk generator (apache#1312) HDDS-4140. Auto-close /pending pull requests after 21 days of inactivity (apache#1344) HDDS-4152. Archive container logs for kubernetes check (apache#1355) HDDS-4056. Convert OzoneAdmin to pluggable model (apache#1285) HDDS-3972. Add option to limit number of items displaying through ldb tool. (apache#1206) HDDS-4068. Client should not retry same OM on network connection failure (apache#1324) HDDS-4062. Non rack aware pipelines should not be created if multiple racks are alive. (apache#1291) ...
What changes were proposed in this pull request?
OzoneAdmin
is a collection of loosely coupled admin tools. This change converts it to the pluggable model introduced in HDDS-4046, where subcommands register themselves declaratively, instead of the parent command listing each subcommand.--scm
parameter fromozone admin
command to the subcommands that actually need it. Currently all exceptOMAdmin
do, but this is likely to change by adding moreom
and other non-scm
subcommands in the future. This is made easier by:--scm
-related logic to the newScmOption
mixin,ScmClient
-based subcommands (similar toHandler
forozone shell
subcommands).MIXEE
, but useful anyway.)Note that moving the
--scm
parameter is backwards incompatible. Previously the following command was valid:and this one was not:
but now it's the other way around. The same applies to all SCM-related admin commands. If necessary, we could provide backwards compatibility, but I don't think it's worth the effort and extra code complexity. By default SCM address is read from config, so command-line override should be rare.
https://issues.apache.org/jira/browse/HDDS-4056
How was this patch tested?
Existing acceptance tests cover some of the commands. Also added new test cases for several
ozone admin
subcommands.CI pending