You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/06/22 10:23:09 UTC
[GitHub] [hadoop-ozone] elek opened a new pull request #1105: HDDS-3828. Configuration parsing of ozone insight should be based on fields
elek opened a new pull request #1105:
URL: https://github.com/apache/hadoop-ozone/pull/1105
## What changes were proposed in this pull request?
Ozone insight command can print out configuration related to a specific ozone component with parsing the @Config object.
But the usage of config annotations has been changed since HDDS-2661. We should check the annotated fields not methods.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3828
## How was this patch tested?
With docker-compose based test:
https://www.youtube.com/watch?v=ZLknuMIl_jw&list=PLCaV-jpCBO8UK5Ged2A_iv3eHuozzMsYv&index=5&t=3s
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] dineshchitlangia merged pull request #1105: HDDS-3828. Configuration parsing of ozone insight should be based on fields
Posted by GitBox <gi...@apache.org>.
dineshchitlangia merged pull request #1105:
URL: https://github.com/apache/hadoop-ozone/pull/1105
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on a change in pull request #1105: HDDS-3828. Configuration parsing of ozone insight should be based on fields
Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1105:
URL: https://github.com/apache/hadoop-ozone/pull/1105#discussion_r443568885
##########
File path: hadoop-ozone/insight/src/main/java/org/apache/hadoop/ozone/insight/ConfigurationSubCommand.java
##########
@@ -71,9 +71,9 @@ private void showConfig(Class clazz, Type type) {
String prefix = configGroup.prefix();
- for (Method method : clazz.getMethods()) {
- if (method.isAnnotationPresent(Config.class)) {
- Config config = method.getAnnotation(Config.class);
+ for (Field field : clazz.getDeclaredFields()) {
Review comment:
Good point.
But I am afraid we need a unit test to here to prove if it works as it's a more complex logic. I added one to prove it work.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1105: HDDS-3828. Configuration parsing of ozone insight should be based on fields
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1105:
URL: https://github.com/apache/hadoop-ozone/pull/1105#discussion_r443489367
##########
File path: hadoop-ozone/insight/src/main/java/org/apache/hadoop/ozone/insight/ConfigurationSubCommand.java
##########
@@ -71,9 +71,9 @@ private void showConfig(Class clazz, Type type) {
String prefix = configGroup.prefix();
- for (Method method : clazz.getMethods()) {
- if (method.isAnnotationPresent(Config.class)) {
- Config config = method.getAnnotation(Config.class);
+ for (Field field : clazz.getDeclaredFields()) {
Review comment:
Shouldn't it handle inherited fields, too? I don't think we have such a configuration object hierarchy currently, but injection is prepared for it:
https://github.com/apache/hadoop-ozone/blob/53549eea6f5dfc8e4cc9594583d7510b206a8acd/hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java#L37-L44
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org