From 0f38d5858e0aa0b53dbdc2df896fbedda9543835 Mon Sep 17 00:00:00 2001
From: stxue1 <stxue@ucsc.edu>
Date: Fri, 8 Nov 2024 11:21:14 -0800
Subject: [PATCH] Don't keep FTP objects around forever and disable FTP SSL by
 default

---
 docs/appendices/environment_vars.rst     |  4 ++--
 src/toil/jobStores/abstractJobStore.py   | 27 ++++++++----------------
 src/toil/{jobStores => lib}/ftp_utils.py |  0
 3 files changed, 11 insertions(+), 20 deletions(-)
 rename src/toil/{jobStores => lib}/ftp_utils.py (100%)

diff --git a/docs/appendices/environment_vars.rst b/docs/appendices/environment_vars.rst
index 561ff4cb5a..f1d700d49a 100644
--- a/docs/appendices/environment_vars.rst
+++ b/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   |
diff --git a/src/toil/jobStores/abstractJobStore.py b/src/toil/jobStores/abstractJobStore.py
index e5e8ee6d84..37b9b23a09 100644
--- a/src/toil/jobStores/abstractJobStore.py
+++ b/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())))
diff --git a/src/toil/jobStores/ftp_utils.py b/src/toil/lib/ftp_utils.py
similarity index 100%
rename from src/toil/jobStores/ftp_utils.py
rename to src/toil/lib/ftp_utils.py