[
https://issues.apache.org/jira/browse/MAPREDUCE-5889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14010323#comment-14010323
]
Tsuyoshi OZAWA commented on MAPREDUCE-5889:
-------------------------------------------
Thanks for the patch, Akira. My comments are as follows:
1. IMHO, it's not good idea to use {{@SuppressWarnings("deprecation")}}
basically if a method uses deprecated methods. It can be good mark to refactor.
Could you explain why you add the annotation to {{createValueAggregatorJob}}
and {{getConf}}?
2. We shouldn't delete the test cases for {{addInputPaths(Job job, Path...
inputPaths)}} for backward compatibility. How about adding tests against
{{addInputPaths(Job, Path...)}}?
3. If we add {{addInputPaths(Job job, Path... inputPaths)}}, why don't you call
it in {{addInputPaths(Job, String)}} like {{setInputPaths(Job, String)}}?
{code}
public static void addInputPaths(JobConf conf, String commaSeparatedPaths) {
addInputPaths(conf, StringUtils.stringToPath(
getPathStrings(commaSeparatedPaths)));
}
{code}
> Deprecate FileInputFormat.setInputPaths(Job, String) and
> FileInputFormat.addInputPaths(Job, String)
> ---------------------------------------------------------------------------------------------------
>
> Key: MAPREDUCE-5889
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-5889
> Project: Hadoop Map/Reduce
> Issue Type: Improvement
> Reporter: Akira AJISAKA
> Assignee: Akira AJISAKA
> Priority: Minor
> Labels: newbie
> Attachments: MAPREDUCE-5889.patch, MAPREDUCE-5889.patch
>
>
> {{FileInputFormat.setInputPaths(Job job, String commaSeparatedPaths)}} and
> {{FileInputFormat.addInputPaths(Job job, String commaSeparatedPaths)}} fail
> to parse commaSeparatedPaths if a comma is included in the file path. (e.g.
> Path: {{/path/file,with,comma}})
> We should deprecate these methods and document to use {{setInputPaths(Job
> job, Path... inputPaths)}} and {{addInputPaths(Job job, Path... inputPaths)}}
> instead.
--
This message was sent by Atlassian JIRA
(v6.2#6252)