[
https://issues.apache.org/jira/browse/AMBARI-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13564954#comment-13564954
]
Sumit Mohanty commented on AMBARI-1260:
---------------------------------------
LGTM. Few nits.
AbstractProviderModule.java
Line 320 - Should we move the port numbers to their own constants - e.g.
DEFAULT_NAME_NODE_PORT - if you take this change there are 4 other values in
this section.
Line 381 - Should the log contain the clusterName, serviceName, etc. for easy
diagnosis?
Line 385 - Should we define a constant string for "version1" and use that here.
Line 387 - The loop will continue to overwrite versionTag for every non-null
configs. Did you intend to break after the first non-null value?
Line 396 - We can move the var declaration to just before the if
(configResources != null) check at 412
Line 409 - Should the log contain the clusterName, configType, configTag, etc.
for easy diagnosis?
LIne 413 - It also seems that the loop will overwrite values for keys if there
are multiple config resources. If we expect only one config resource should we
assert that?
JMXPropertyProviderTest.java
Are there unused imports?
Do we need additional unit tests that actually read from mock config files to
test the service and configResource readers used by AbstractProviderModule? Any
need for negative test cases? (unless they already exist)
> Remove hard coded JMX port mappings
> -----------------------------------
>
> Key: AMBARI-1260
> URL: https://issues.apache.org/jira/browse/AMBARI-1260
> Project: Ambari
> Issue Type: Improvement
> Components: controller
> Affects Versions: 1.2.1
> Reporter: Siddharth Wagle
> Assignee: Siddharth Wagle
> Attachments: AMBARI-1260.patch
>
>
> The JMXPropertyProvider contains a map of component names to ports ...
> {code}
> JMX_PORTS.put("NAMENODE", "50070");
> JMX_PORTS.put("DATANODE", "50075");
> JMX_PORTS.put("JOBTRACKER", "50030");
> JMX_PORTS.put("TASKTRACKER", "50060");
> JMX_PORTS.put("HBASE_MASTER", "60010");
> {code}
> These ports can change in configuration. Need to create the mapping
> dynamically.
> This is required for secure HDP cluster to work.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
