Skip to content
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-12058. Use CommandLine out/err in GenericCli subclasses #7673

Merged
merged 3 commits into from
Jan 11, 2025

Conversation

adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

picocli allows setting custom out/err streams to make testing easier. This PR updates subclasses of GenericCli (except OzoneRepair, which is being changed in #7653) to use these custom streams instead of System.out/err. (Further PRs will take care of sets of subcommands to avoid too large patches.)

https://issues.apache.org/jira/browse/HDDS-12058

How was this patch tested?

Tested some of the commands manually:

$ bin/ozone getconf -Dasdf=qwerty confKey asdf
qwerty

$ bin/ozone checknative                       
Native library checking:
hadoop:  false 
ISA-L:   false 
rocks-tools: false 

$ bin/ozone genconf /tmp
ozone-site.xml has been generated at /tmp

CI:
https://github.com/adoroszlai/ozone/actions/runs/12697388661

@adoroszlai adoroszlai self-assigned this Jan 9, 2025
@adoroszlai adoroszlai added the code-cleanup Changes that aim to make code better, without changing functionality. label Jan 10, 2025
Copy link
Contributor

@nandakumar131 nandakumar131 left a comment

Choose a reason for hiding this comment

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

Thanks @adoroszlai for the improvement. Overall the change looks good to me.
Will it make sense to add helper methods in GenericCli to print the output and error messages? You can ignore this if you feel otherwise.

@adoroszlai
Copy link
Contributor Author

Will it make sense to add helper methods in GenericCli to print the output and error messages?

Thanks @nandakumar131 for the suggestion. Added out() and err() in GenericCli for easier access to the writers.

@adoroszlai adoroszlai merged commit b89b6e0 into apache:master Jan 11, 2025
42 checks passed
@adoroszlai adoroszlai deleted the HDDS-12058 branch January 11, 2025 19:36
@adoroszlai
Copy link
Contributor Author

Thanks @nandakumar131 for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-cleanup Changes that aim to make code better, without changing functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants