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 14:37:20 UTC
[GitHub] [hadoop-ozone] adoroszlai opened a new pull request #1106: HDDS-2413. Set configuration variables from annotated java objects
adoroszlai opened a new pull request #1106:
URL: https://github.com/apache/hadoop-ozone/pull/1106
## What changes were proposed in this pull request?
Allow setting `Configuration` from strongly-typed config objects (ie. ones annotated with `@Config`). This is the opposite direction of existing `ConfigurationSource.getObject()`.
One caveat is that the config object needs to be created via `getObject()` for default values from annotations to be properly set.
https://issues.apache.org/jira/browse/HDDS-2413
## How was this patch tested?
Added unit test. Also updated a single part of "real" code where this change makes string constants unnecessary (`DUFactory.Conf` and `TestDUFactory`).
https://github.com/adoroszlai/hadoop-ozone/runs/795413474
----------------------------------------------------------------
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 merged pull request #1106: HDDS-2413. Set configuration variables from annotated java objects
Posted by GitBox <gi...@apache.org>.
elek merged pull request #1106:
URL: https://github.com/apache/hadoop-ozone/pull/1106
----------------------------------------------------------------
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 #1106: HDDS-2413. Set configuration variables from annotated java objects
Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1106:
URL: https://github.com/apache/hadoop-ozone/pull/1106#discussion_r447520786
##########
File path: hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java
##########
@@ -37,9 +39,6 @@
char[] getPassword(String key) throws IOException;
- @Deprecated
Review comment:
I really don't like the approach when the current port number (after using :0) is added to the configuration. It's easier if the configuration is immutable after loading.
I understand that the immutable is not possible with this injection and I accept that it's necessary.
One possible approach is to introduce two interfaces: `ConfigSource` (read-only) and `ConfigDestination` (write only). `OzoneConfiguration` can implement both. But there could be a specific `ConfigSource` (eg. `EnvironmentVariableConfigSource`) which would be read only.
But I am fine with committing this patch as is, but interested about opinion.
Also: it seems to be useful for testing to introduce an `OzoneConfiguration.fromAnnotatedObject()` method.
----------------------------------------------------------------
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 #1106: HDDS-2413. Set configuration variables from annotated java objects
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1106:
URL: https://github.com/apache/hadoop-ozone/pull/1106#discussion_r448345001
##########
File path: hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java
##########
@@ -37,9 +39,6 @@
char[] getPassword(String key) throws IOException;
- @Deprecated
Review comment:
> One possible approach is to introduce two interfaces: `ConfigSource` (read-only) and `ConfigDestination` (write only). `OzoneConfiguration` can implement both.
Thanks @elek for the suggestion. I was considering this, but needed this nudge to implement it.
----------------------------------------------------------------
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 #1106: HDDS-2413. Set configuration variables from annotated java objects
Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1106:
URL: https://github.com/apache/hadoop-ozone/pull/1106#discussion_r448490975
##########
File path: hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java
##########
@@ -37,9 +39,6 @@
char[] getPassword(String key) throws IOException;
- @Deprecated
Review comment:
> I was considering this, but needed this nudge to implement it.
Oh, I see why. It makes the patch bigger because we use this dirty hack to store the port in the config.
----------------------------------------------------------------
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 pull request #1106: HDDS-2413. Set configuration variables from annotated java objects
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1106:
URL: https://github.com/apache/hadoop-ozone/pull/1106#issuecomment-652547442
Thanks @elek for reviewing and committing it.
----------------------------------------------------------------
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