Xiao Chen added a comment - 05/Jan/17 23:57 Attaching patch 1 to make them configurable. Locally ran the contract tests, found some of them failing even without this fix. Tracking that in HADOOP-13954 .

Hadoop QA added a comment - 06/Jan/17 08:52 -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 12m 24s trunk passed +1 compile 10m 1s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 1m 11s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 1m 43s trunk passed +1 javadoc 0m 51s trunk passed +1 mvninstall 0m 43s the patch passed +1 compile 10m 14s the patch passed +1 javac 10m 14s the patch passed +1 checkstyle 0m 31s hadoop-common-project/hadoop-common: The patch generated 0 new + 6 unchanged - 1 fixed = 6 total (was 7) +1 mvnsite 1m 4s the patch passed +1 mvneclipse 0m 18s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply -1 findbugs 1m 38s hadoop-common-project/hadoop-common generated 6 new + 0 unchanged - 0 fixed = 6 total (was 0) +1 javadoc 0m 52s the patch passed -1 unit 9m 2s hadoop-common in the patch failed. +1 asflicense 0m 33s The patch does not generate ASF License warnings. 53m 55s Reason Tests FindBugs module:hadoop-common-project/hadoop-common   Comparison of String objects using == or != in org.apache.hadoop.fs.ftp.FTPFileSystem.getTransferMode(Configuration) At FTPFileSystem.java:== or != in org.apache.hadoop.fs.ftp.FTPFileSystem.getTransferMode(Configuration) At FTPFileSystem.java: [line 186]   Comparison of String objects using == or != in org.apache.hadoop.fs.ftp.FTPFileSystem.getTransferMode(Configuration) At FTPFileSystem.java:== or != in org.apache.hadoop.fs.ftp.FTPFileSystem.getTransferMode(Configuration) At FTPFileSystem.java: [line 183]   Comparison of String objects using == or != in org.apache.hadoop.fs.ftp.FTPFileSystem.getTransferMode(Configuration) At FTPFileSystem.java:== or != in org.apache.hadoop.fs.ftp.FTPFileSystem.getTransferMode(Configuration) At FTPFileSystem.java: [line 181]   Comparison of String objects using == or != in org.apache.hadoop.fs.ftp.FTPFileSystem.setDataConnectionMode(FTPClient, Configuration) At FTPFileSystem.java:== or != in org.apache.hadoop.fs.ftp.FTPFileSystem.setDataConnectionMode(FTPClient, Configuration) At FTPFileSystem.java: [line 218]   Comparison of String objects using == or != in org.apache.hadoop.fs.ftp.FTPFileSystem.setDataConnectionMode(FTPClient, Configuration) At FTPFileSystem.java:== or != in org.apache.hadoop.fs.ftp.FTPFileSystem.setDataConnectionMode(FTPClient, Configuration) At FTPFileSystem.java: [line 213]   Comparison of String objects using == or != in org.apache.hadoop.fs.ftp.FTPFileSystem.setDataConnectionMode(FTPClient, Configuration) At FTPFileSystem.java:== or != in org.apache.hadoop.fs.ftp.FTPFileSystem.setDataConnectionMode(FTPClient, Configuration) At FTPFileSystem.java: [line 215] Failed junit tests hadoop.ha.TestZKFailoverController Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13953 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12845897/HADOOP-13953.01.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f779c49ba721 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 4a659ff Default Java 1.8.0_111 findbugs v3.0.0 whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11379/artifact/patchprocess/whitespace-eol.txt findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/11379/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html unit https://builds.apache.org/job/PreCommit-HADOOP-Build/11379/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11379/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11379/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Hadoop QA added a comment - 06/Jan/17 19:00 +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 12m 50s trunk passed +1 compile 9m 38s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 1m 1s trunk passed +1 mvneclipse 0m 18s trunk passed +1 findbugs 1m 24s trunk passed +1 javadoc 0m 48s trunk passed +1 mvninstall 0m 36s the patch passed +1 compile 9m 7s the patch passed +1 javac 9m 7s the patch passed +1 checkstyle 0m 28s hadoop-common-project/hadoop-common: The patch generated 0 new + 6 unchanged - 1 fixed = 6 total (was 7) +1 mvnsite 0m 58s the patch passed +1 mvneclipse 0m 17s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 32s the patch passed +1 javadoc 0m 47s the patch passed +1 unit 8m 29s hadoop-common in the patch passed. +1 asflicense 0m 33s The patch does not generate ASF License warnings. 51m 15s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13953 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846048/HADOOP-13953.02.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ee6a5c3031a2 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2977bc6 Default Java 1.8.0_111 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11381/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11381/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Wei-Chiu Chuang added a comment - 06/Jan/17 19:56 Thanks Xiao Chen for filing the bug report and contributing the patch. Overall looks good to me. Two nit: LOG.info( "Cannot parse the value for " + FS_FTP_DATA_CONNECTION_MODE + ": " + mode + ". Using default ." ); Shouldn't it be a WARN log? It would also be nice to add the valid configuration values, and default values into core-default.xml

Xiao Chen added a comment - 06/Jan/17 21:26 Thanks for the review Wei-Chiu Chuang . Patch 3 to address the comments. Didn't know those ftp configs are in core-default.xml, added there. Also looked for docs but found none.

Hadoop QA added a comment - 06/Jan/17 22:22 -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 13m 20s trunk passed +1 compile 10m 25s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 1m 2s trunk passed +1 mvneclipse 0m 19s trunk passed +1 findbugs 1m 26s trunk passed +1 javadoc 0m 47s trunk passed +1 mvninstall 0m 37s the patch passed +1 compile 9m 45s the patch passed +1 javac 9m 45s the patch passed +1 checkstyle 0m 29s hadoop-common-project/hadoop-common: The patch generated 0 new + 6 unchanged - 1 fixed = 6 total (was 7) +1 mvnsite 1m 0s the patch passed +1 mvneclipse 0m 18s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 1m 36s the patch passed +1 javadoc 0m 47s the patch passed -1 unit 8m 1s hadoop-common in the patch failed. +1 asflicense 0m 32s The patch does not generate ASF License warnings. 52m 54s Reason Tests Failed junit tests hadoop.conf.TestCommonConfigurationFields   hadoop.fs.viewfs.TestViewFileSystemLocalFileSystem Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13953 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846085/HADOOP-13953.03.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux a94f4161e2f4 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 2977bc6 Default Java 1.8.0_111 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/11386/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11386/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11386/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Xiao Chen added a comment - 07/Jan/17 02:19 Patch 4 for the test failure. IMO those should be skipped rather than added to CommonConfigurationKeys - the existing 2 defined there aren't used anywhere either.

Hadoop QA added a comment - 07/Jan/17 04:10 +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 12m 44s trunk passed +1 compile 10m 4s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 1m 6s trunk passed +1 mvneclipse 0m 18s trunk passed +1 findbugs 1m 33s trunk passed +1 javadoc 0m 50s trunk passed +1 mvninstall 0m 40s the patch passed +1 compile 10m 26s the patch passed +1 javac 10m 26s the patch passed +1 checkstyle 0m 31s hadoop-common-project/hadoop-common: The patch generated 0 new + 11 unchanged - 1 fixed = 11 total (was 12) +1 mvnsite 1m 5s the patch passed +1 mvneclipse 0m 18s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 1m 43s the patch passed +1 javadoc 0m 52s the patch passed +1 unit 8m 38s hadoop-common in the patch passed. +1 asflicense 0m 33s The patch does not generate ASF License warnings. 53m 52s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13953 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846151/HADOOP-13953.04.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 383910e40f7f 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / a59df15 Default Java 1.8.0_111 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11389/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11389/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

Wei-Chiu Chuang added a comment - 08/Jan/17 13:38 - edited +1

Xiao Chen added a comment - 08/Jan/17 17:43 Thanks a lot, Wei-Chiu! -Xiao

Hudson added a comment - 09/Jan/17 23:55 FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #11092 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11092/ ) HADOOP-13953 . Make FTPFileSystem's data connection mode and transfer (weichiu: rev 0a212a40fcbd12a11294bff7a31e7433111733c9) (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/ftp/TestFTPFileSystem.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestCommonConfigurationFields.java (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ftp/FTPFileSystem.java

Wei-Chiu Chuang added a comment - 10/Jan/17 18:37 Committed the patch to trunk and branch-2. Thanks again for Xiao Chen 's contribution!