Don't keep FTP objects around forever and disable FTP SSL by default

docs/appendices/environment_vars.rst

@@ -225,8 +225,8 @@ There are several environment variables that affect the way Toil runs.
 +----------------------------------+----------------------------------------------------+
 | TOIL_FTP_USE_SSL                 | Enable or disable usage of SSL for connecting to   |
 |                                  | FTP servers                                        |
-|                                  | (``True`` by default).                             |
-|                                  | Example: ``TOIL_FTP_USE_SSL=False``                |
+|                                  | (``False`` by default).                            |
+|                                  | Example: ``TOIL_FTP_USE_SSL=True``                 |
 +----------------------------------+----------------------------------------------------+
 | TOIL_WES_BROKER_URL              | An optional broker URL to use to communicate       |
 |                                  | between the WES server and Celery task queue. If   |

src/toil/jobStores/abstractJobStore.py

@@ -46,7 +46,7 @@
     JobException,
     ServiceJobDescription,
 )
-from toil.jobStores.ftp_utils import FtpFsAccess
+from toil.lib.ftp_utils import FtpFsAccess
 from toil.lib.compatibility import deprecated
 from toil.lib.conversions import strtobool
 from toil.lib.io import WriteWatchingStream
@@ -1874,12 +1874,9 @@ class JobStoreSupport(AbstractJobStore, metaclass=ABCMeta):
     stores.
     """
 
-    ftp = None
-
     @classmethod
-    def _setup_ftp(cls) -> None:
-        if cls.ftp is None:
-            cls.ftp = FtpFsAccess(insecure=strtobool(os.environ.get('TOIL_FTP_USE_SSL', 'True')) is False)
+    def _setup_ftp(cls) -> FtpFsAccess:
+        return FtpFsAccess(insecure=strtobool(os.environ.get('TOIL_FTP_USE_SSL', 'False')) is False)
 
     @classmethod
     def _supports_url(cls, url: ParseResult, export: bool = False) -> bool:
@@ -1889,10 +1886,8 @@ def _supports_url(cls, url: ParseResult, export: bool = False) -> bool:
     def _url_exists(cls, url: ParseResult) -> bool:
         # Deal with FTP first to support user/password auth
         if url.scheme.lower() == "ftp":
-            cls._setup_ftp()
-            # mypy is unable to understand that ftp must exist by this point
-            assert cls.ftp is not None
-            return cls.ftp.exists(url.geturl())
+            ftp = cls._setup_ftp()
+            return ftp.exists(url.geturl())
 
         try:
             with closing(urlopen(Request(url.geturl(), method="HEAD"))):
@@ -1911,10 +1906,8 @@ def _url_exists(cls, url: ParseResult) -> bool:
     )
     def _get_size(cls, url: ParseResult) -> Optional[int]:
         if url.scheme.lower() == "ftp":
-            cls._setup_ftp()
-            # mypy is unable to understand that ftp must exist by this point
-            assert cls.ftp is not None
-            return cls.ftp.size(url.geturl())
+            ftp = cls._setup_ftp()
+            return ftp.size(url.geturl())
 
         # just read the header for content length
         resp = urlopen(Request(url.geturl(), method="HEAD"))
@@ -1954,11 +1947,9 @@ def count(l: int) -> None:
     def _open_url(cls, url: ParseResult) -> IO[bytes]:
         # Deal with FTP first so we support user/password auth
         if url.scheme.lower() == "ftp":
-            cls._setup_ftp()
-            # mypy is unable to understand that ftp must exist by this point
-            assert cls.ftp is not None
+            ftp = cls._setup_ftp()
             # we open in read mode as write mode is not supported
-            return cls.ftp.open(url.geturl(), mode="r")
+            return ftp.open(url.geturl(), mode="r")
 
         try:
             return cast(IO[bytes], closing(urlopen(url.geturl())))