[
https://issues.apache.org/jira/browse/CRUNCH-649?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16021273#comment-16021273
]
Attila Sasvari commented on CRUNCH-649:
---------------------------------------
[~pairg] Thanks for the patch. In general it looks good to me, with git apply I
could apply the patch and unit tests passed.
Some comments:
* As I can see {{FileUtil.copyMerge()}} was removed from hadoop-3. Changeset is
based on [org.apache.hadoop.fs.FileUtil #
copyMerge()|https://github.com/apache/hadoop/blob/branch-2.6/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java#L382]
and [checkDest()|
https://github.com/apache/hadoop/blob/branch-2.6/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java#L494]
of hadoop 2.6
* {{dstFS.exists(dstFile) && dstFS.getFileStatus(dstFile).isDirectory()}} is
duplicated and could be extracted into a separate method (isExistingDirectory()
or something).
* nit: a whitespace is missing in copyMerge(). Please change
FROM:
{code}
if (dstFS.exists(dstFile) && dstFS.getFileStatus(dstFile).isDirectory()){
{code}
TO:
{code}
if (dstFS.exists(dstFile) && dstFS.getFileStatus(dstFile).isDirectory()) {
{code}
* Can we add some tests that covers this method? I am thinking of having
something like
[TestFileUtil|https://github.com/apache/hadoop/blob/branch-2.6/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java#L504]
in hadoop 2.6.
* When I tried to apply CRUNCH-649_v1.patch following
https://cwiki.apache.org/confluence/display/CRUNCH/How+To+Contribute
{noformat}
$ git am CRUNCH-649_v1.patch
Patch format
{noformat}
> Support Hadoop 3 in Crunch-Spark
> --------------------------------
>
> Key: CRUNCH-649
> URL: https://issues.apache.org/jira/browse/CRUNCH-649
> Project: Crunch
> Issue Type: Sub-task
> Components: Spark
> Affects Versions: 1.0.0
> Reporter: Gergő Pásztor
> Assignee: Gergő Pásztor
> Fix For: 1.0.0
>
> Attachments: CRUNCH-649_v1.patch
>
>
> Support Hadoop 3 in Crunch-Spark
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
