[
https://issues.apache.org/jira/browse/YARN-4514?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15232863#comment-15232863
]
Varun Saxena commented on YARN-4514:
------------------------------------
[~leftnoteasy], the patch as such looks fine to me.
I have a couple of comments though.
For the config "localBaseUrl" , we have to configure it with a slash(/) in the
end. Otherwise URL would not be formed properly. A user can easily miss
configuring it that way if he changes corsproxy address for instance. It is
more likely that somebody would configure it as localhost:1337 instead of
localhost:1337/
Although it can be mentioned in config file comments but it just looks a bit
weird.
I think in each of the adapter files we can put a / by ourselves.
Moreover the config names have been named as URLs'. timelineWebUrl, rmWebUrl,
etc. URL contains a protocol identifier too. Maybe name it as
timelineWebAddress, rmWebAddress and so on.
What do you think ?
Also I had one earlier comment. Do you think namespaces should be put in
configuration file. Shouldnt they be constants ? Sunil was of the opinion that
this would make it easier when we are upgrading. You can take a call on it. I
felt these can be constants.
These are minor comments though and can be taken care later while doing some
other JIRA. If its blocking merge to trunk, I think you can go ahead and commit
it.
> [YARN-3368] Cleanup hardcoded configurations, such as RM/ATS addresses
> ----------------------------------------------------------------------
>
> Key: YARN-4514
> URL: https://issues.apache.org/jira/browse/YARN-4514
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Wangda Tan
> Assignee: Sunil G
> Attachments: YARN-4514-YARN-3368.1.patch,
> YARN-4514-YARN-3368.2.patch, YARN-4514-YARN-3368.3.patch,
> YARN-4514-YARN-3368.4.patch
>
>
> We have several configurations are hard-coded, for example, RM/ATS addresses,
> we should make them configurable.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
