Merge ~pappacena/txpkgupload:dtp-timeout-config into txpkgupload:master

[Thiago F. Pappacena]

Adding config parameter for dtp timeout

[William Grant (wgrant)]

If something can't connect within 10 seconds, I'm not sure we care. That's almost always going to be a firewall problem -- even the Moon is only a 3s round trip.

review: Disapprove

[Thiago F. Pappacena (pappacena)]

The idea was not to have this timeout increased in production, but rather to let a sys admin do, in production, the test they needed to do.

Anyway, I'm closing this MP.

Unmerged commits

6daa850... by Thiago F. Pappacena on 2020-11-09

Adding config parameter for dtp timeout

diff --git a/etc/txpkgupload.yaml b/etc/txpkgupload.yaml
index 45f7a99..ae94b52 100644
--- a/etc/txpkgupload.yaml
+++ b/etc/txpkgupload.yaml
@@ -40,6 +40,9 @@ oops:
 ## disconnected.
 # idle_timeout: 3600
 
+## DTP connection timeout for FTP connections (defaults to 10).
+dtp_timeout: 15
+
 ## Where on the filesystem do uploads live?
 # fsroot: "/var/tmp/txpkgupload/incoming"
 
diff --git a/src/txpkgupload/plugin.py b/src/txpkgupload/plugin.py
index 0168643..c93933e 100644
--- a/src/txpkgupload/plugin.py
+++ b/src/txpkgupload/plugin.py
@@ -123,6 +123,9 @@ class Config(Schema):
     # disconnected.
     idle_timeout = Int(if_missing=3600)
 
+    # DTP timeout for FTP connections.
+    dtp_timeout = Int(if_missing=10)
+
     # Where on the filesystem do uploads live?
     fsroot = String(if_missing=None)
 
@@ -238,7 +241,8 @@ class PkgUploadServiceMaker:
             port=ftp_config["port"],
             root=root,
             temp_dir=temp_dir,
-            idle_timeout=config["idle_timeout"])
+            idle_timeout=config["idle_timeout"],
+            dtp_timeout=config["dtp_timeout"])
         ftp_service.name = "ftp"
         ftp_service.setServiceParent(services)
 
diff --git a/src/txpkgupload/tests/test_plugin.py b/src/txpkgupload/tests/test_plugin.py
index 162c289..9413fe8 100644
--- a/src/txpkgupload/tests/test_plugin.py
+++ b/src/txpkgupload/tests/test_plugin.py
@@ -111,6 +111,7 @@ class TestConfig(TestCase):
                 "port": 2121,
                 },
             "idle_timeout": 3600,
+            "dtp_timeout": 10,
             "oops": {
                 "directory": "",
                 "reporter": "PKGUPLOAD",
@@ -164,6 +165,11 @@ class TestConfig(TestCase):
             {"idle_timeout": "bob"},
             "idle_timeout: Please enter an integer value")
 
+    def test_option_dtp_timeout_integer(self):
+        self.check_exception(
+            {"dtp_timeout": "bob"},
+            "dtp_timeout: Please enter an integer value")
+
 
 class TestOptions(TestCase):
     """Tests for `txpkgupload.plugin.Options`."""
@@ -521,6 +527,9 @@ class TestPkgUploadServiceMakerMixin:
         self.assertEqual(
             len(service.namedServices), len(service.services),
             "Not all services are named.")
+        # Make sure we are using the configurations from the config file.
+        ftp_service = service.services[0]
+        self.assertEqual(15, ftp_service.factory.protocol.dtpTimeout)
 
     def _uploadPath(self, path):
         """Return system path of specified path inside an upload.
diff --git a/src/txpkgupload/twistedftp.py b/src/txpkgupload/twistedftp.py
index 37deffb..8e58b32 100644
--- a/src/txpkgupload/twistedftp.py
+++ b/src/txpkgupload/twistedftp.py
@@ -360,7 +360,7 @@ class FTPWithEPSV(ftp.FTP):
 class FTPServiceFactory(service.Service):
     """A factory that makes an `FTPService`"""
 
-    def __init__(self, port, root, temp_dir, idle_timeout):
+    def __init__(self, port, root, temp_dir, idle_timeout, dtp_timeout):
         realm = FTPRealm(root, temp_dir)
         portal = Portal(realm)
         portal.registerChecker(AccessCheck())
@@ -368,6 +368,7 @@ class FTPServiceFactory(service.Service):
 
         factory.tld = root
         factory.protocol = FTPWithEPSV
+        factory.protocol.dtpTimeout = dtp_timeout
         factory.welcomeMessage = "Launchpad upload server"
         factory.timeOut = idle_timeout
 
@@ -375,7 +376,8 @@ class FTPServiceFactory(service.Service):
         self.portno = port
 
     @staticmethod
-    def makeFTPService(port, root, temp_dir, idle_timeout):
+    def makeFTPService(port, root, temp_dir, idle_timeout, dtp_timeout):
         strport = "tcp6:%s" % port
-        factory = FTPServiceFactory(port, root, temp_dir, idle_timeout)
+        factory = FTPServiceFactory(
+            port, root, temp_dir, idle_timeout, dtp_timeout)
         return strports.service(strport, factory.ftpfactory)