Browse Source

Support timeouts for HTTP client operations

Jelmer Vernooij 1 month ago
parent
commit
e43b167076
3 changed files with 95 additions and 7 deletions
  1. 6 0
      NEWS
  2. 30 7
      dulwich/client.py
  3. 59 0
      tests/test_client.py

+ 6 - 0
NEWS

@@ -1,5 +1,9 @@
 0.23.1	UNRELEASED
 
+ * Support ``untracked_files="normal"`` argument to ``porcelain.status``,
+   and make this the default.
+   (Jelmer Vernooij, #835)
+
  * Return symrefs from ls_refs. (Jelmer Vernooij, #863)
 
  * Support short commit hashes in ``porcelain.reset()``.
@@ -38,6 +42,8 @@
  * Add support for includes in configuration files.
    (#1216, Jelmer Vernooij)
 
+ * Support timeouts for HTTP client operations.  (Jelmer Vernooij)
+
 0.23.0	2025-06-21
 
  * Add basic ``rebase`` subcommand. (Jelmer Vernooij)

+ 30 - 7
dulwich/client.py

@@ -2325,6 +2325,7 @@ def default_urllib3_manager(
     pool_manager_cls=None,
     proxy_manager_cls=None,
     base_url=None,
+    timeout=None,
     **override_kwargs,
 ) -> Union["urllib3.ProxyManager", "urllib3.PoolManager"]:
     """Return urllib3 connection pool manager.
@@ -2333,6 +2334,7 @@ def default_urllib3_manager(
 
     Args:
       config: `dulwich.config.ConfigDict` instance with Git configuration.
+      timeout: Timeout for HTTP requests in seconds
       override_kwargs: Additional arguments for `urllib3.ProxyManager`
 
     Returns:
@@ -2376,6 +2378,15 @@ def default_urllib3_manager(
         except KeyError:
             ca_certs = None
 
+        # Check for timeout configuration
+        if timeout is None:
+            try:
+                timeout = config.get(b"http", b"timeout")
+                if timeout is not None:
+                    timeout = int(timeout)
+            except KeyError:
+                pass
+
     if user_agent is None:
         user_agent = default_user_agent_string()
 
@@ -2384,6 +2395,10 @@ def default_urllib3_manager(
     kwargs = {
         "ca_certs": ca_certs,
     }
+
+    # Add timeout if specified
+    if timeout is not None:
+        kwargs["timeout"] = timeout
     if ssl_verify is True:
         kwargs["cert_reqs"] = "CERT_REQUIRED"
     elif ssl_verify is False:
@@ -2930,13 +2945,17 @@ class Urllib3HttpGitClient(AbstractHttpGitClient):
         config=None,
         username=None,
         password=None,
+        timeout=None,
         **kwargs,
     ) -> None:
         self._username = username
         self._password = password
+        self._timeout = timeout
 
         if pool_manager is None:
-            self.pool_manager = default_urllib3_manager(config, base_url=base_url)
+            self.pool_manager = default_urllib3_manager(
+                config, base_url=base_url, timeout=timeout
+            )
         else:
             self.pool_manager = pool_manager
 
@@ -2969,14 +2988,18 @@ class Urllib3HttpGitClient(AbstractHttpGitClient):
         req_headers["Pragma"] = "no-cache"
 
         try:
+            request_kwargs = {
+                "headers": req_headers,
+                "preload_content": False,
+            }
+            if self._timeout is not None:
+                request_kwargs["timeout"] = self._timeout
+
             if data is None:
-                resp = self.pool_manager.request(
-                    "GET", url, headers=req_headers, preload_content=False
-                )
+                resp = self.pool_manager.request("GET", url, **request_kwargs)
             else:
-                resp = self.pool_manager.request(
-                    "POST", url, headers=req_headers, body=data, preload_content=False
-                )
+                request_kwargs["body"] = data
+                resp = self.pool_manager.request("POST", url, **request_kwargs)
         except urllib3.exceptions.HTTPError as e:
             raise GitProtocolError(str(e)) from e
 

+ 59 - 0
tests/test_client.py

@@ -1411,6 +1411,39 @@ class HttpGitClientTests(TestCase):
         self.assertEqual(pack_data[4:8], b"\x00\x00\x00\x02")  # version 2
         self.assertEqual(pack_data[8:12], b"\x00\x00\x00\x01")  # 1 object
 
+    def test_timeout_configuration(self) -> None:
+        """Test that timeout parameter is properly configured."""
+        url = "https://github.com/jelmer/dulwich"
+        timeout = 30
+
+        c = HttpGitClient(url, timeout=timeout)
+        self.assertEqual(c._timeout, timeout)
+
+    def test_timeout_from_config(self) -> None:
+        """Test that timeout can be configured via git config."""
+        from dulwich.config import ConfigDict
+
+        url = "https://github.com/jelmer/dulwich"
+        config = ConfigDict()
+        config.set((b"http",), b"timeout", b"25")
+
+        c = HttpGitClient(url, config=config)
+        # The timeout should be set on the pool manager
+        # Since we can't easily access the timeout from the pool manager,
+        # we just verify the client was created successfully
+        self.assertIsNotNone(c.pool_manager)
+
+    def test_timeout_parameter_precedence(self) -> None:
+        """Test that explicit timeout parameter takes precedence over config."""
+        from dulwich.config import ConfigDict
+
+        url = "https://github.com/jelmer/dulwich"
+        config = ConfigDict()
+        config.set((b"http",), b"timeout", b"25")
+
+        c = HttpGitClient(url, config=config, timeout=15)
+        self.assertEqual(c._timeout, 15)
+
 
 class TCPGitClientTests(TestCase):
     def test_get_url(self) -> None:
@@ -1670,6 +1703,32 @@ class DefaultUrllib3ManagerTest(TestCase):
         manager = default_urllib3_manager(config=None, cert_reqs="CERT_NONE")
         self.assertEqual(manager.connection_pool_kw["cert_reqs"], "CERT_NONE")
 
+    def test_timeout_parameter(self) -> None:
+        """Test that timeout parameter is passed to urllib3 manager."""
+        timeout = 30
+        manager = default_urllib3_manager(config=None, timeout=timeout)
+        self.assertEqual(manager.connection_pool_kw["timeout"], timeout)
+
+    def test_timeout_from_config(self) -> None:
+        """Test that timeout can be configured via git config."""
+        from dulwich.config import ConfigDict
+
+        config = ConfigDict()
+        config.set((b"http",), b"timeout", b"25")
+
+        manager = default_urllib3_manager(config=config)
+        self.assertEqual(manager.connection_pool_kw["timeout"], 25)
+
+    def test_timeout_parameter_precedence(self) -> None:
+        """Test that explicit timeout parameter takes precedence over config."""
+        from dulwich.config import ConfigDict
+
+        config = ConfigDict()
+        config.set((b"http",), b"timeout", b"25")
+
+        manager = default_urllib3_manager(config=config, timeout=15)
+        self.assertEqual(manager.connection_pool_kw["timeout"], 15)
+
 
 class SubprocessSSHVendorTests(TestCase):
     def setUp(self) -> None: