You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2020/04/10 20:10:02 UTC

[GitHub] [hadoop] mpryahin opened a new pull request #1952: HDFS-1820 FTPFileSystem attempts to close the outputstream even when it is not initialised

mpryahin opened a new pull request #1952: HDFS-1820 FTPFileSystem attempts to close the outputstream even when it is not initialised
URL: https://github.com/apache/hadoop/pull/1952
 
 
   - Making sure an underlying outputstream is successfully created by apache-commons FTPClient before wrapping it with FSDataOutputStream.  
   - Gracefully release resources when a destination file can't be created due to lack of permissions. 
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.
URL: https://github.com/apache/hadoop/pull/1952#issuecomment-612596354
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 35s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  20m 36s |  trunk passed  |
   | +1 :green_heart: |  compile  |  18m 14s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 52s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 33s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  17m  3s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  1s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   2m  7s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   2m  5s |  trunk passed  |
   | -0 :warning: |  patch  |   2m 27s |  Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  compile  |  16m 16s |  the patch passed  |
   | +1 :green_heart: |  javac  |  16m 16s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 48s |  hadoop-common-project/hadoop-common: The patch generated 1 new + 7 unchanged - 0 fixed = 8 total (was 7)  |
   | +1 :green_heart: |  mvnsite  |   1m 28s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  2s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  shadedclient  |  14m 20s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  4s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   2m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   9m 23s |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 55s |  The patch does not generate ASF License warnings.  |
   |  |   | 110m 40s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1952/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1952 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
   | uname | Linux dca168ee9c84 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 8d49229 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1952/2/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1952/2/testReport/ |
   | Max. process+thread count | 3339 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1952/2/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] goiri commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.
URL: https://github.com/apache/hadoop/pull/1952#discussion_r407100202
 
 

 ##########
 File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/ftp/TestFTPFileSystem.java
 ##########
 @@ -37,9 +54,71 @@
  */
 public class TestFTPFileSystem {
 
+  private TestFtpServer server;
+
   @Rule
   public Timeout testTimeout = new Timeout(180000);
 
+  @Before
+  public void setUp() throws Exception {
+    server = new TestFtpServer(GenericTestUtils.getTestDir().toPath()).start();
+  }
+
+  @After
+  @SuppressWarnings("ResultOfMethodCallIgnored")
+  public void tearDown() throws Exception {
+    server.stop();
+    Files.walk(server.getFtpRoot())
+        .sorted(Comparator.reverseOrder())
+        .map(java.nio.file.Path::toFile)
+        .forEach(File::delete);
+  }
+
+  @Test
+  public void testCreateWithWritePermissions() throws Exception {
+    BaseUser user = server.addUser("test", "password", new WritePermission());
+    Configuration configuration = new Configuration();
+    configuration.set("fs.defaultFS", "ftp:///");
+    configuration.set("fs.ftp.host", "localhost");
+    configuration.setInt("fs.ftp.host.port", server.getPort());
+    configuration.set("fs.ftp.user.localhost", user.getName());
+    configuration.set("fs.ftp.password.localhost", user.getPassword());
+    configuration.set("fs.ftp.impl.disable.cache", "true");
+
+    FileSystem fs = FileSystem.get(configuration);
+    byte[] bytesExpected = "hello world".getBytes(StandardCharsets.UTF_8);
+    try (FSDataOutputStream outputStream = fs.create(new Path("test1.txt"))) {
+      outputStream.write(bytesExpected);
+    }
+    try (FSDataInputStream input = fs.open(new Path("test1.txt"))) {
+      assertThat(bytesExpected, equalTo(IOUtils.readFullyToByteArray(input)));
+    }
+  }
+
+  @Test
+  public void testCreateWithoutWritePermissions() throws Exception {
+    BaseUser user = server.addUser("test", "password");
+    Configuration configuration = new Configuration();
+    configuration.set("fs.defaultFS", "ftp:///");
+    configuration.set("fs.ftp.host", "localhost");
+    configuration.setInt("fs.ftp.host.port", server.getPort());
+    configuration.set("fs.ftp.user.localhost", user.getName());
+    configuration.set("fs.ftp.password.localhost", user.getPassword());
+    configuration.set("fs.ftp.impl.disable.cache", "true");
+
+    FileSystem fs = FileSystem.get(configuration);
+    byte[] bytesExpected = "hello world".getBytes(StandardCharsets.UTF_8);
+
+    try (FSDataOutputStream outputStream = fs.create(new Path("test1.txt"))) {
+      outputStream.write(bytesExpected);
 
 Review comment:
   We should use LambdaTestUtils#intercept and make sure that we fail after write()

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mpryahin commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.

Posted by GitBox <gi...@apache.org>.
mpryahin commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.
URL: https://github.com/apache/hadoop/pull/1952#discussion_r407166330
 
 

 ##########
 File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/ftp/TestFTPFileSystem.java
 ##########
 @@ -37,9 +54,71 @@
  */
 public class TestFTPFileSystem {
 
+  private TestFtpServer server;
+
   @Rule
   public Timeout testTimeout = new Timeout(180000);
 
+  @Before
+  public void setUp() throws Exception {
+    server = new TestFtpServer(GenericTestUtils.getTestDir().toPath()).start();
+  }
+
+  @After
+  @SuppressWarnings("ResultOfMethodCallIgnored")
+  public void tearDown() throws Exception {
+    server.stop();
+    Files.walk(server.getFtpRoot())
+        .sorted(Comparator.reverseOrder())
+        .map(java.nio.file.Path::toFile)
+        .forEach(File::delete);
+  }
+
+  @Test
+  public void testCreateWithWritePermissions() throws Exception {
+    BaseUser user = server.addUser("test", "password", new WritePermission());
+    Configuration configuration = new Configuration();
+    configuration.set("fs.defaultFS", "ftp:///");
+    configuration.set("fs.ftp.host", "localhost");
+    configuration.setInt("fs.ftp.host.port", server.getPort());
+    configuration.set("fs.ftp.user.localhost", user.getName());
+    configuration.set("fs.ftp.password.localhost", user.getPassword());
+    configuration.set("fs.ftp.impl.disable.cache", "true");
 
 Review comment:
   indeed! thank you for pointing out

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mpryahin commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.

Posted by GitBox <gi...@apache.org>.
mpryahin commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.
URL: https://github.com/apache/hadoop/pull/1952#discussion_r407166378
 
 

 ##########
 File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/ftp/TestFTPFileSystem.java
 ##########
 @@ -37,9 +54,71 @@
  */
 public class TestFTPFileSystem {
 
+  private TestFtpServer server;
+
   @Rule
   public Timeout testTimeout = new Timeout(180000);
 
+  @Before
+  public void setUp() throws Exception {
+    server = new TestFtpServer(GenericTestUtils.getTestDir().toPath()).start();
+  }
+
+  @After
+  @SuppressWarnings("ResultOfMethodCallIgnored")
+  public void tearDown() throws Exception {
+    server.stop();
+    Files.walk(server.getFtpRoot())
+        .sorted(Comparator.reverseOrder())
+        .map(java.nio.file.Path::toFile)
+        .forEach(File::delete);
+  }
+
+  @Test
+  public void testCreateWithWritePermissions() throws Exception {
+    BaseUser user = server.addUser("test", "password", new WritePermission());
+    Configuration configuration = new Configuration();
+    configuration.set("fs.defaultFS", "ftp:///");
+    configuration.set("fs.ftp.host", "localhost");
+    configuration.setInt("fs.ftp.host.port", server.getPort());
+    configuration.set("fs.ftp.user.localhost", user.getName());
+    configuration.set("fs.ftp.password.localhost", user.getPassword());
+    configuration.set("fs.ftp.impl.disable.cache", "true");
+
+    FileSystem fs = FileSystem.get(configuration);
+    byte[] bytesExpected = "hello world".getBytes(StandardCharsets.UTF_8);
+    try (FSDataOutputStream outputStream = fs.create(new Path("test1.txt"))) {
+      outputStream.write(bytesExpected);
+    }
+    try (FSDataInputStream input = fs.open(new Path("test1.txt"))) {
+      assertThat(bytesExpected, equalTo(IOUtils.readFullyToByteArray(input)));
+    }
+  }
+
+  @Test
+  public void testCreateWithoutWritePermissions() throws Exception {
+    BaseUser user = server.addUser("test", "password");
+    Configuration configuration = new Configuration();
+    configuration.set("fs.defaultFS", "ftp:///");
+    configuration.set("fs.ftp.host", "localhost");
+    configuration.setInt("fs.ftp.host.port", server.getPort());
+    configuration.set("fs.ftp.user.localhost", user.getName());
+    configuration.set("fs.ftp.password.localhost", user.getPassword());
+    configuration.set("fs.ftp.impl.disable.cache", "true");
+
+    FileSystem fs = FileSystem.get(configuration);
+    byte[] bytesExpected = "hello world".getBytes(StandardCharsets.UTF_8);
+
+    try (FSDataOutputStream outputStream = fs.create(new Path("test1.txt"))) {
+      outputStream.write(bytesExpected);
 
 Review comment:
   fixed, thank you!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] goiri commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.
URL: https://github.com/apache/hadoop/pull/1952#discussion_r407100095
 
 

 ##########
 File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/ftp/TestFTPFileSystem.java
 ##########
 @@ -37,9 +54,71 @@
  */
 public class TestFTPFileSystem {
 
+  private TestFtpServer server;
+
   @Rule
   public Timeout testTimeout = new Timeout(180000);
 
+  @Before
+  public void setUp() throws Exception {
+    server = new TestFtpServer(GenericTestUtils.getTestDir().toPath()).start();
+  }
+
+  @After
+  @SuppressWarnings("ResultOfMethodCallIgnored")
+  public void tearDown() throws Exception {
+    server.stop();
+    Files.walk(server.getFtpRoot())
+        .sorted(Comparator.reverseOrder())
+        .map(java.nio.file.Path::toFile)
+        .forEach(File::delete);
+  }
+
+  @Test
+  public void testCreateWithWritePermissions() throws Exception {
+    BaseUser user = server.addUser("test", "password", new WritePermission());
+    Configuration configuration = new Configuration();
+    configuration.set("fs.defaultFS", "ftp:///");
+    configuration.set("fs.ftp.host", "localhost");
+    configuration.setInt("fs.ftp.host.port", server.getPort());
+    configuration.set("fs.ftp.user.localhost", user.getName());
+    configuration.set("fs.ftp.password.localhost", user.getPassword());
+    configuration.set("fs.ftp.impl.disable.cache", "true");
 
 Review comment:
   Can we use setBoolean()?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.
URL: https://github.com/apache/hadoop/pull/1952#issuecomment-612602618
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m 10s |  trunk passed  |
   | +1 :green_heart: |  compile  |  19m  4s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 49s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 25s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  16m 28s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  1s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   2m  7s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   2m  4s |  trunk passed  |
   | -0 :warning: |  patch  |   2m 25s |  Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 51s |  the patch passed  |
   | +1 :green_heart: |  compile  |  16m 17s |  the patch passed  |
   | +1 :green_heart: |  javac  |  16m 17s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 50s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 26s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  1s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  shadedclient  |  14m 16s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   2m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   9m 17s |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 52s |  The patch does not generate ASF License warnings.  |
   |  |   | 109m  3s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1952/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1952 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
   | uname | Linux e2a7c82e97d7 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 8d49229 |
   | Default Java | 1.8.0_242 |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1952/3/testReport/ |
   | Max. process+thread count | 2620 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1952/3/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] goiri commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.

Posted by GitBox <gi...@apache.org>.
goiri commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.
URL: https://github.com/apache/hadoop/pull/1952#discussion_r407100428
 
 

 ##########
 File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/ftp/TestFtpServer.java
 ##########
 @@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.hadoop.fs.ftp;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.Authority;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.impl.DefaultFtpServer;
+import org.apache.ftpserver.listener.Listener;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+
+/**
+ * Helper class facilitating to manage a local ftp
+ * server for unit tests purposes only.
+ */
+public class TestFtpServer {
 
 Review comment:
   Starting by Test in  this folder I would expect it to have a @Test in it.
   TBH, I don't know what better name to give it, FtpTestServer?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mpryahin commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.

Posted by GitBox <gi...@apache.org>.
mpryahin commented on a change in pull request #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.
URL: https://github.com/apache/hadoop/pull/1952#discussion_r407166505
 
 

 ##########
 File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/ftp/TestFtpServer.java
 ##########
 @@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.hadoop.fs.ftp;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+
+import org.apache.ftpserver.FtpServer;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.ftplet.Authority;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.UserManager;
+import org.apache.ftpserver.impl.DefaultFtpServer;
+import org.apache.ftpserver.listener.Listener;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.usermanager.PropertiesUserManagerFactory;
+import org.apache.ftpserver.usermanager.impl.BaseUser;
+
+/**
+ * Helper class facilitating to manage a local ftp
+ * server for unit tests purposes only.
+ */
+public class TestFtpServer {
 
 Review comment:
   Thank you, renamed  the class into `FtpTestServer`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1952: HDFS-1820. FTPFileSystem attempts to close the outputstream even when it is not initialised.
URL: https://github.com/apache/hadoop/pull/1952#issuecomment-612242452
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 34s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m 22s |  trunk passed  |
   | +1 :green_heart: |  compile  |  16m 50s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 48s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 28s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  16m 37s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  3s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   2m  8s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   2m  6s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 54s |  the patch passed  |
   | +1 :green_heart: |  compile  |  16m 16s |  the patch passed  |
   | +1 :green_heart: |  javac  |  16m 16s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 49s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 26s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  xml  |   0m  1s |  The patch has no ill-formed XML file.  |
   | +1 :green_heart: |  shadedclient  |  13m 57s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  3s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   2m 13s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   9m 20s |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 54s |  The patch does not generate ASF License warnings.  |
   |  |   | 106m 54s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1952/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1952 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle |
   | uname | Linux 164fa4dd4ebd 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 275c478 |
   | Default Java | 1.8.0_242 |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1952/1/testReport/ |
   | Max. process+thread count | 1822 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1952/1/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org