crunch-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Attila Sasvari (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CRUNCH-649) Support Hadoop 3 in Crunch-Spark
Date Tue, 23 May 2017 14:46:04 GMT

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

Mime
View raw message