adoroszlai commented on code in PR #9152:
URL: https://github.com/apache/ozone/pull/9152#discussion_r2503996287


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/HddsConfServlet.java:
##########
@@ -41,13 +53,53 @@
 public class HddsConfServlet extends HttpServlet {
 
   private static final long serialVersionUID = 1L;
+  private static final Logger LOG = 
LoggerFactory.getLogger(HddsConfServlet.class);
 
   protected static final String FORMAT_JSON = "json";
   protected static final String FORMAT_XML = "xml";
   private static final String COMMAND = "cmd";
   private static final OzoneConfiguration OZONE_CONFIG =
       new OzoneConfiguration();
 
+  /**
+   * Represents metadata for a configuration property, including its name, 
value and description.
+   */
+  public static class PropertyMetadata {

Review Comment:
   Can we reuse `OzoneConfiguration.Property` instead of adding a new class?
   
   
https://github.com/apache/ozone/blob/3f54b1452c5b725431c69ca25e0bb2a3d400894e/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java#L148



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/HddsConfServlet.java:
##########
@@ -165,6 +226,92 @@ private void processConfigTagRequest(HttpServletRequest 
request, String cmd,
 
   }
 
+  /**
+   * Build a map of property names to descriptions by reading from 
configuration resources.
+   * @param config the OzoneConfiguration to extract descriptions from
+   * @return map of property name to description
+   */
+  private Map<String, String> buildDescriptionMap(OzoneConfiguration config) {
+    Map<String, String> descriptionMap = new HashMap<>();
+    
+    // List of configuration resource names to check
+    String[] configResources = new String[] {
+        "core-default.xml",
+        "core-site.xml",
+        "hdfs-default.xml",
+        "hdfs-site.xml",
+        "hdds-common-default.xml",
+        "hdds-client-default.xml",
+        "hdds-container-service-default.xml",
+        "hdds-server-framework-default.xml",
+        "hdds-server-scm-default.xml",
+        "ozone-common-default.xml",
+        "ozone-csi-default.xml",
+        "ozone-manager-default.xml",
+        "ozone-recon-default.xml",
+        "ozone-default.xml",
+        "ozone-site.xml"
+    };

Review Comment:
   Please define the list of resources in `OzoneConfiguration` and use it in 
both places, here and in:
   
   
https://github.com/apache/ozone/blob/3f54b1452c5b725431c69ca25e0bb2a3d400894e/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java#L213



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/conf/HddsConfServlet.java:
##########
@@ -165,6 +226,92 @@ private void processConfigTagRequest(HttpServletRequest 
request, String cmd,
 
   }
 
+  /**
+   * Build a map of property names to descriptions by reading from 
configuration resources.
+   * @param config the OzoneConfiguration to extract descriptions from
+   * @return map of property name to description
+   */
+  private Map<String, String> buildDescriptionMap(OzoneConfiguration config) {
+    Map<String, String> descriptionMap = new HashMap<>();
+    
+    // List of configuration resource names to check
+    String[] configResources = new String[] {
+        "core-default.xml",
+        "core-site.xml",
+        "hdfs-default.xml",
+        "hdfs-site.xml",
+        "hdds-common-default.xml",
+        "hdds-client-default.xml",
+        "hdds-container-service-default.xml",
+        "hdds-server-framework-default.xml",
+        "hdds-server-scm-default.xml",
+        "ozone-common-default.xml",
+        "ozone-csi-default.xml",
+        "ozone-manager-default.xml",
+        "ozone-recon-default.xml",
+        "ozone-default.xml",
+        "ozone-site.xml"
+    };
+    
+    for (String resourceName : configResources) {
+      try {
+        URL resourceUrl = config.getResource(resourceName);
+        if (resourceUrl != null) {
+          parseXmlDescriptions(resourceUrl, descriptionMap);
+        }
+      } catch (Exception e) {
+        LOG.error("Could not read descriptions from resource: {}", 
resourceName, e);
+      }
+    }
+
+    return descriptionMap;
+  }
+
+  /**
+   * Parse XML configuration file and extract property descriptions using DOM 
parser.
+   * @param resourceUrl URL of the XML resource to parse
+   * @param descriptionMap map to populate with property name -> description 
mappings
+   */
+  private void parseXmlDescriptions(URL resourceUrl, Map<String, String> 
descriptionMap) {
+    try (InputStream inputStream = resourceUrl.openStream()) {
+      DocumentBuilderFactory factory = 
XMLUtils.newSecureDocumentBuilderFactory();
+      DocumentBuilder builder = factory.newDocumentBuilder();

Review Comment:
   Please reuse the factory and builder in `buildDescriptionMap` for parsing 
all files.



-- 
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]

Reply via email to