knox-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sandeep More (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (KNOX-1064) Externalize Hadoop Service Configuration Details and Service URL Creation
Date Tue, 03 Oct 2017 16:05:00 GMT

    [ 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)

Mime
View raw message