adoroszlai commented on code in PR #4369:
URL: https://github.com/apache/ozone/pull/4369#discussion_r1131055734
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java:
##########
@@ -802,7 +804,13 @@ public static final class Port {
*/
public enum Name {
STANDALONE, RATIS, REST, REPLICATION, RATIS_ADMIN, RATIS_SERVER,
- RATIS_DATASTREAM;
+ RATIS_DATASTREAM,
Review Comment:
`RATIS_DATASTREAM` is also a new port after 1.3.0, so shouldn't it be
annotated?
(We can split the task into two (1. introducing backwards-compatible
handling of ports, 2. adding the new http ports) if you prefer.)
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java:
##########
@@ -802,7 +804,13 @@ public static final class Port {
*/
public enum Name {
STANDALONE, RATIS, REST, REPLICATION, RATIS_ADMIN, RATIS_SERVER,
- RATIS_DATASTREAM;
+ RATIS_DATASTREAM,
+ @BelongsToHDDSLayoutVersion(HDDSLayoutFeature
+ .WEBUI_PORTS_IN_DATANODEDETAILS)
+ HTTP,
Review Comment:
nit: for readability, please add static import for
`WEBUI_PORTS_IN_DATANODEDETAILS`
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/DatanodeIdYaml.java:
##########
@@ -219,11 +226,31 @@ public void setCurrentVersion(int version) {
}
private static DatanodeDetailsYaml getDatanodeDetailsYaml(
- DatanodeDetails datanodeDetails) {
+ DatanodeDetails datanodeDetails, ConfigurationSource conf)
+ throws IOException {
+
+ DatanodeLayoutStorage datanodeLayoutStorage
+ = new DatanodeLayoutStorage(conf, datanodeDetails.getUuidString());
Map<String, Integer> portDetails = new LinkedHashMap<>();
if (!CollectionUtils.isEmpty(datanodeDetails.getPorts())) {
for (DatanodeDetails.Port port : datanodeDetails.getPorts()) {
+ Field f = null;
+ try {
+ f = port.getName().getClass()
+ .getDeclaredField(port.getName().toString());
Review Comment:
nit: use `Enum.name()` instead of `Enum.toString()`, which may be overridden
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/DatanodeIdYaml.java:
##########
@@ -219,11 +226,31 @@ public void setCurrentVersion(int version) {
}
private static DatanodeDetailsYaml getDatanodeDetailsYaml(
- DatanodeDetails datanodeDetails) {
+ DatanodeDetails datanodeDetails, ConfigurationSource conf)
+ throws IOException {
+
+ DatanodeLayoutStorage datanodeLayoutStorage
+ = new DatanodeLayoutStorage(conf, datanodeDetails.getUuidString());
Map<String, Integer> portDetails = new LinkedHashMap<>();
if (!CollectionUtils.isEmpty(datanodeDetails.getPorts())) {
for (DatanodeDetails.Port port : datanodeDetails.getPorts()) {
+ Field f = null;
+ try {
+ f = port.getName().getClass()
+ .getDeclaredField(port.getName().toString());
+ } catch (NoSuchFieldException e) {
+ e.printStackTrace();
Review Comment:
I think we should add a `Logger` in `DatanodeIdYaml` and log this as an
error (I guess it really shouldn't happen).
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
