Repository: PyFilesystem/pyfilesystem2

Issue:
fs.ftpfs.FTPFS.upload opens new connections
Hi,
I'm looking at this function:
https://github.com/PyFilesystem/pyfilesystem2/blob/64d7a52e1f3037f33937f91e9db812cafdbbafc0/fs/ftpfs.py#L813

`_manage_ftp` always opens a new FTP connection, together with user login, "FEAT" command and so on. This is done everytime one wants to copy a file to an FTPFS. Is there a good reason why that code doesn't just use `self._ftp`, the connection that should be open for the lifetime of the FTPFS object? Otherwise, I'd like to create a PR to fix that. 

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 31cb320..4dfe905 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Make `FTPFile`, `MemoryFile` and `RawWrapper` accept [`array.array`](https://docs.python.org/3/library/array.html)
   arguments for the `write` and `writelines` methods, as expected by their base class [`io.RawIOBase`](https://docs.python.org/3/library/io.html#io.RawIOBase).
 - Various documentation issues, including `MemoryFS` docstring not rendering properly.
+- Avoid creating a new connection on every call of `FTPFS.upload`. Closes [#455](https://github.com/PyFilesystem/pyfilesystem2/issues/455).
 
 
 ## [2.4.12] - 2021-01-14
diff --git a/fs/ftpfs.py b/fs/ftpfs.py
index 3d8b094..c5929ee 100644
--- a/fs/ftpfs.py
+++ b/fs/ftpfs.py
@@ -814,11 +814,10 @@ class FTPFS(FS):
         # type: (Text, BinaryIO, Optional[int], **Any) -> None
         _path = self.validatepath(path)
         with self._lock:
-            with self._manage_ftp() as ftp:
-                with ftp_errors(self, path):
-                    ftp.storbinary(
-                        str("STOR ") + _encode(_path, self.ftp.encoding), file
-                    )
+            with ftp_errors(self, path):
+                self.ftp.storbinary(
+                    str("STOR ") + _encode(_path, self.ftp.encoding), file
+                )
 
     def writebytes(self, path, contents):
         # type: (Text, ByteString) -> None


Test patch:
diff --git a/tests/test_ftpfs.py b/tests/test_ftpfs.py
index b414283..b0e0c1f 100644
--- a/tests/test_ftpfs.py
+++ b/tests/test_ftpfs.py
@@ -12,7 +12,12 @@ import time
 import unittest
 import uuid
 
-from six import text_type
+try:
+    from unittest import mock
+except ImportError:
+    import mock
+
+from six import text_type, BytesIO
 
 from ftplib import error_perm
 from ftplib import error_temp
@@ -275,6 +280,12 @@ class TestFTPFS(FSTestCases, unittest.TestCase):
         with open_fs(url, create=True) as ftp_fs:
             self.assertTrue(ftp_fs.isfile("foo"))
 
+    def test_upload_connection(self):
+        with mock.patch.object(self.fs, "_manage_ftp") as _manage_ftp:
+            self.fs.upload("foo", BytesIO(b"hello"))
+        self.assertEqual(self.fs.gettext("foo"), "hello")
+        _manage_ftp.assert_not_called()
+
 
 class TestFTPFSNoMLSD(TestFTPFS):
     def make_fs(self):