[
https://issues.apache.org/jira/browse/KNOX-1064?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16189905#comment-16189905
]
Sandeep More commented on KNOX-1064:
------------------------------------
Great, thanks for the patch Phil, sorry for the delay in reviewing.
Since the patch is a bit longer (for me) I reviewed the entire file in some
cases than the diff as it was easier to get the context with full file. Also,
the suggestion I have are not major they are minor nit-picks patch overall
looks great (I haven't yet tested it)
Comments:
1. in AmbariCluster class, serviceConfigurations can be a ConcurrentHashMap
insted of HashMap
2. AmbariComponent class, any reason why the methods were made package
protected ?
3. AmbariServiceDiscovery class,
3.1 serviceConfiguration Map is not used
3.2 Given that AMBARI_CLUSTERS_URI and AMBARI_HOSTROLES_URI are Ambari
specific paths which could change in th future, do you think we can move it to
ext. mapping file
3.3 aliasService variable is not used
3.4 You can remove the comment on line # 147
4. SimpleDescriptorHandler class
4.1 line # 25 and 28 have * imports.
4.2 line # 124, may be use BufferedWriter to wrap FileWriter to be more
effecient
4.3 The Writer needs to be closed in finally block to prevent leaking in
case of exception.
5 AmbariDynamicServiceURLCreator class
5.1 mappingConfigFile file object should be closed.
5.2 at line # 64, we could close FileInputStream after it's dome reading
5.3 at line 69, same as 5.2
> Externalize Hadoop Service Configuration Details and Service URL Creation
> -------------------------------------------------------------------------
>
> Key: KNOX-1064
> URL: https://issues.apache.org/jira/browse/KNOX-1064
> Project: Apache Knox
> Issue Type: Bug
> Components: Server
> Affects Versions: 0.14.0
> Reporter: Phil Zampino
> Assignee: Phil Zampino
> Labels: kip-8
> Attachments: KNOX-1064-001.patch, KNOX-1064-002.patch,
> KNOX-1064-003.patch, KNOX-1064.patch
>
>
> For the sake of flexibility in terms of the services supported by service
> discovery and topology generation, the following should be externalized from
> the code:
> # The set and mapping of service components and their respective
> configuration details.
> # The service URL generation details.
> The goal is to be able to add/modify these details without having to modify
> source code.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
