[
https://issues.apache.org/jira/browse/HADOOP-6105?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12751809#action_12751809
]
Hemanth Yamijala commented on HADOOP-6105:
------------------------------------------
The changes look mostly fine. I have just a few minor comments:
- "adds the deprecated key to the deprecation map." -> Capitalize 'adds'
- addDeprecation should provide a way to specify custom message.
- Throw an IllegalArgumentException if key or value paramaters are invalid.
- getWarningMessage should cache the default message after it has built it.
- [Issue] - should we warn every time in get ? Will it prove to be too much
information like we discovered when we did MAPREDUCE-40 ? One option could be
to log everytime in set. Log once in processDeprecatedKeys() to cover get.
Thoughts ?
- "It returns the value of the new key, if the old key is deprecated." -
suggest rewording slightly to "If the key is deprecated, it returns the value
of first key that replaces the deprecated key." Also, use one message
consistently for all APIs.
- I would suggest set doesn't recursively call set again for the replacement
keys. We can avoid some additional function calls and lookups that way. This
means replacement keys can't be deprecated, but I think that's a very
reasonable assumption to make.
- It will be good to check the consistency of get for old and new keys for all
cases in the test case.
- Need one test case for testing mapping of one key to more than one new keys.
This must be tested for both set and get. I would suggest a new test case for
this.
> Provide a way to automatically handle backward compatibility of deprecated
> keys
> -------------------------------------------------------------------------------
>
> Key: HADOOP-6105
> URL: https://issues.apache.org/jira/browse/HADOOP-6105
> Project: Hadoop Common
> Issue Type: Improvement
> Components: conf
> Reporter: Hemanth Yamijala
> Assignee: V.V.Chaitanya Krishna
> Attachments: HADOOP-6105-1.patch, HADOOP-6105-2.patch,
> HADOOP-6105-3.patch, HADOOP-6105-4.patch, HADOOP-6105-5.patch,
> HADOOP-6105-6.patch, HADOOP-6105-7.patch, HADOOP-6105-8.patch,
> HADOOP-6105.patch, HADOOP-6105.patch
>
>
> There are cases when we have had to deprecate configuration keys. Use cases
> include, changing the names of variables to better match intent, splitting a
> single parameter into two - for maps, reduces etc.
> In such cases, we typically provide a backwards compatible option for the old
> keys. The handling of such cases might typically be common enough to actually
> add support for it in a generic fashion in the Configuration class. Some
> initial discussion around this started in HADOOP-5919, but since the project
> split happened in between we decided to open this issue to fix it in common.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
