From d74e4488bfbac709a6aaeb9b760203a56c32420e Mon Sep 17 00:00:00 2001 From: Andreas Wundsam Date: Mon, 13 Oct 2025 14:30:44 +0200 Subject: [PATCH 1/2] Add retry support to pybsn.connect() for improved reliability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Network connections can fail transiently due to temporary issues. This adds a retries parameter to enable automatic retry behavior with exponential backoff, improving reliability of HTTP clients without requiring manual HTTPAdapter configuration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- README.md | 57 +++ examples/retry_example.py | 97 +++++ pybsn/__init__.py | 17 + test/test_retry_bigdbclient.py | 404 +++++++++++++++++++++ test/test_retry_bigdbclient_integration.py | 285 +++++++++++++++ test/test_retry_connect.py | 117 ++++++ 6 files changed, 977 insertions(+) create mode 100644 examples/retry_example.py create mode 100644 test/test_retry_bigdbclient.py create mode 100644 test/test_retry_bigdbclient_integration.py create mode 100644 test/test_retry_connect.py diff --git a/README.md b/README.md index 7d88465c..d6532d2f 100644 --- a/README.md +++ b/README.md @@ -177,6 +177,63 @@ root.core.switch_config.post({ "name": "leaf-1a", "mac-address": "01:02:03:04:05 root.core.switch.match(name="leaf-1a").disconnect.rpc() ``` +--- +## Retry Configuration + +pybsn supports automatic retry of failed HTTP requests to improve reliability when connecting to BigDB. You can configure retry behavior using the `retries` parameter in `pybsn.connect()`. + +### Simple Retry Count + +Pass an integer to specify the total number of retry attempts: + +```python +# Retry up to 3 times on failures +client = pybsn.connect(host="controller", token="", retries=3) +``` + +By default, only idempotent HTTP methods (GET, HEAD, OPTIONS, TRACE) are automatically retried. Non-idempotent methods (POST, PUT, PATCH, DELETE) are not retried to prevent unintended side effects. + +### Advanced Retry Configuration + +For more control over retry behavior, use a `urllib3.util.retry.Retry` object: + +```python +from urllib3.util.retry import Retry + +# Configure custom retry behavior +retry_config = Retry( + total=5, # Total retry attempts + backoff_factor=1, # Exponential backoff (1s, 2s, 4s, 8s, 16s) + status_forcelist=[503, 504], # Retry on specific HTTP status codes +) + +client = pybsn.connect(host="controller", token="", retries=retry_config) +``` + +### Retry Non-Idempotent Methods + +**Use with caution!** To retry non-idempotent methods (POST, PUT, PATCH, DELETE), explicitly configure `allowed_methods`: + +```python +from urllib3.util.retry import Retry + +# WARNING: Only use if your operations are truly idempotent! +retry_config = Retry( + total=3, + backoff_factor=0.5, + status_forcelist=[503], + allowed_methods=["GET", "POST", "PUT", "PATCH", "DELETE"], # Include all methods +) + +client = pybsn.connect(host="controller", token="", retries=retry_config) +``` + +### Default Behavior + +By default (`retries=None`), no automatic retries are performed. Requests fail immediately on errors, preserving backward compatibility with existing code. + +For more examples, see `examples/retry_example.py`. + --- ## Contributing diff --git a/examples/retry_example.py b/examples/retry_example.py new file mode 100644 index 00000000..cce8ed50 --- /dev/null +++ b/examples/retry_example.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python +""" +Example demonstrating retry configuration with pybsn.connect() + +This example shows how to configure automatic retries for failed HTTP requests +to improve reliability when connecting to BigDB. +""" +import argparse + +from urllib3.util.retry import Retry + +import pybsn + +parser = argparse.ArgumentParser(description="Demonstrate retry configuration") +parser.add_argument("--host", "-H", type=str, default="127.0.0.1", help="Controller IP/Hostname to connect to") +parser.add_argument("--user", "-u", type=str, default="admin", help="Username") +parser.add_argument("--password", "-p", type=str, default="adminadmin", help="Password") + +args = parser.parse_args() + + +def example_simple_retry(): + """Example 1: Simple integer retry count (retries GET requests 3 times)""" + print("Example 1: Simple integer retry count") + client = pybsn.connect(args.host, args.user, args.password, retries=3) + + # This will retry up to 3 times on connection errors or certain HTTP status codes + # By default, only idempotent methods (GET, HEAD, OPTIONS, TRACE) are retried + switches = client.root.core.switch() + print(f" Found {len(switches)} switches") + + +def example_custom_retry(): + """Example 2: Custom Retry object with specific configuration""" + print("\nExample 2: Custom Retry with status codes and backoff") + + # Configure retry with: + # - 5 total retry attempts + # - Exponential backoff with 1 second base delay + # - Retry on specific HTTP status codes (503, 504) + retry_config = Retry( + total=5, + backoff_factor=1, # Wait 1s, 2s, 4s, 8s, 16s between retries + status_forcelist=[503, 504], # Retry on Service Unavailable and Gateway Timeout + ) + + client = pybsn.connect(args.host, args.user, args.password, retries=retry_config) + + # This will retry on 503/504 errors with exponential backoff + switches = client.root.core.switch() + print(f" Found {len(switches)} switches with custom retry config") + + +def example_retry_non_idempotent(): + """Example 3: Retry non-idempotent methods (use with caution!)""" + print("\nExample 3: Retry non-idempotent methods (POST, PUT, PATCH, DELETE)") + + # By default, only safe/idempotent methods are retried + # To retry POST/PUT/PATCH/DELETE, explicitly configure allowed_methods + # WARNING: Only use this if your operations are truly idempotent! + retry_config = Retry( + total=3, + backoff_factor=0.5, + status_forcelist=[503], + allowed_methods=["GET", "POST", "PUT", "PATCH", "DELETE"], # Include non-idempotent methods + ) + + client = pybsn.connect(args.host, args.user, args.password, retries=retry_config) + + # Now POST/PUT/PATCH/DELETE operations will also retry on failures + # Use with caution - ensure your operations are idempotent! + switches = client.root.core.switch() + print(f" Found {len(switches)} switches (with non-idempotent retry enabled)") + + +def example_no_retry(): + """Example 4: No retry (default behavior)""" + print("\nExample 4: No retry (default behavior)") + + # Default behavior - no automatic retries + client = pybsn.connect(args.host, args.user, args.password) + + # Failures will raise immediately without retry + switches = client.root.core.switch() + print(f" Found {len(switches)} switches (no retry)") + + +if __name__ == "__main__": + print("=== pybsn Retry Configuration Examples ===\n") + + # Run all examples + example_simple_retry() + example_custom_retry() + example_retry_non_idempotent() + example_no_retry() + + print("\n=== Examples completed ===") diff --git a/pybsn/__init__.py b/pybsn/__init__.py index aee5a516..a4e4c6c9 100644 --- a/pybsn/__init__.py +++ b/pybsn/__init__.py @@ -10,6 +10,7 @@ import requests import urllib3.util from urllib3.exceptions import InsecureRequestWarning +from urllib3.util.retry import Retry warnings.simplefilter("ignore", InsecureRequestWarning) @@ -23,6 +24,7 @@ JSONPrimitive = Union[None, bool, int, float, str] JSONValue = Union[JSONPrimitive, Dict[str, Any], List[Any]] # Any allows nested structures TimeoutType = Union[None, float, "_ClientTimeout", urllib3.util.Timeout] +RetriesType = Union[None, int, Retry] class _ClientTimeout: @@ -622,6 +624,7 @@ def connect( verify_tls: bool = False, session_headers: Optional[Dict[str, str]] = None, timeout: Optional[Union[float, urllib3.util.Timeout]] = None, + retries: RetriesType = None, ) -> BigDbClient: """Creates a connected BigDb client. @@ -645,6 +648,14 @@ def connect( to wait forever. The timeout value will be used as the default for future operations unless it is changed. + :parameter retries: Optional retry configuration for failed HTTP requests. + None (default) - No automatic retries, preserving existing behavior. + int - Total number of retry attempts (e.g., retries=3). + urllib3.util.retry.Retry - Full retry configuration object for advanced control + (e.g., retries=Retry(total=5, backoff_factor=1, status_forcelist=[500, 502, 503, 504])). + Note: By default, only idempotent methods (GET, HEAD, OPTIONS, TRACE) are retried. + Non-idempotent methods (POST, PUT, PATCH, DELETE) require explicit configuration. + (other parameters for advanced/internal use). :return A connected BigDBClient instance @@ -656,6 +667,12 @@ def connect( for k, v in session_headers.items(): session.headers[k] = v + # Mount HTTPAdapter with retry configuration if specified + if retries is not None: + adapter = requests.adapters.HTTPAdapter(max_retries=retries) + session.mount("http://", adapter) + session.mount("https://", adapter) + url = guess_url(session, host) if login is None: login = (token is None) and username is not None and password is not None diff --git a/test/test_retry_bigdbclient.py b/test/test_retry_bigdbclient.py new file mode 100644 index 00000000..945b179e --- /dev/null +++ b/test/test_retry_bigdbclient.py @@ -0,0 +1,404 @@ +import json +import sys +import unittest + +import requests +import responses +from requests.exceptions import ConnectionError +from urllib3.util.retry import Retry + +import pybsn + +sys.path.append("test") + + +class TestRetryBigDbClient(unittest.TestCase): + """ + Test that retry behavior works correctly across all BigDbClient methods. + """ + + url = "http://127.0.0.1:8080" + + def _login_cb(self, req): + self.assertEqual(json.loads(req.body), {"user": "admin", "password": "somepassword"}) + headers = {"Set-Cookie": "session_cookie=v_8yOI7FI-WKr-5QU6nxoJkVtAu5rKI-; Path=/api/;"} + return ( + 200, + headers, + json.dumps( + { + "success": True, + "session-cookie": "UPhNWlmDN0re8cg9xsqe9QT1QvQTznji", + "error-message": "", + "past-login-info": { + "failed-login-count": 0, + "last-success-login-info": {"host": "127.0.0.1", "timestamp": "2019-05-19T19:16:22.328Z"}, + }, + } + ), + ) + + def _add_login_responses(self): + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/auth/healthy", + json={}, + status=200, + ) + responses.add_callback( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/aaa/session/login", + callback=self._login_cb, + content_type="application/json", + ) + + @responses.activate + def test_get_retries_on_failure(self): + """GET request retries on connection error and succeeds after retry.""" + self._add_login_responses() + + # Configure retry to retry on 503 status + retry_config = Retry(total=3, status_forcelist=[503]) + + # First two GET attempts return 503, third succeeds + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json=[{"name": "switch1"}], + status=200, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + result = client.get("controller/core/switch") + + # Should succeed after retries + self.assertEqual(result, [{"name": "switch1"}]) + + @responses.activate + def test_post_retries_on_failure(self): + """POST request retries on connection error when configured.""" + self._add_login_responses() + + # Configure retry to allow POST retries and retry on 503 + retry_config = Retry(total=3, allowed_methods=["POST"], status_forcelist=[503]) + + # First two POST attempts return 503, third succeeds + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=201, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + response = client.post("controller/core/switch", data={"name": "switch1"}) + + # Should succeed after retries + self.assertEqual(response.status_code, 201) + + @responses.activate + def test_put_retries_on_failure(self): + """PUT request retries on connection error when configured.""" + self._add_login_responses() + + # Configure retry to allow PUT retries and retry on 503 + retry_config = Retry(total=3, allowed_methods=["PUT"], status_forcelist=[503]) + + # First two PUT attempts return 503, third succeeds + responses.add( + responses.PUT, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.PUT, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.PUT, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=200, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + response = client.put("controller/core/switch", data={"name": "switch1"}) + + # Should succeed after retries + self.assertEqual(response.status_code, 200) + + @responses.activate + def test_patch_retries_on_failure(self): + """PATCH request retries on connection error when configured.""" + self._add_login_responses() + + # Configure retry to allow PATCH retries and retry on 503 + retry_config = Retry(total=3, allowed_methods=["PATCH"], status_forcelist=[503]) + + # First two PATCH attempts return 503, third succeeds + responses.add( + responses.PATCH, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.PATCH, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.PATCH, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=200, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + response = client.patch("controller/core/switch", data={"name": "switch1"}) + + # Should succeed after retries + self.assertEqual(response.status_code, 200) + + @responses.activate + def test_delete_retries_on_failure(self): + """DELETE request retries on connection error when configured.""" + self._add_login_responses() + + # Configure retry to allow DELETE retries and retry on 503 + retry_config = Retry(total=3, allowed_methods=["DELETE"], status_forcelist=[503]) + + # First two DELETE attempts return 503, third succeeds + responses.add( + responses.DELETE, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.DELETE, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.DELETE, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + status=204, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + response = client.delete("controller/core/switch") + + # Should succeed after retries + self.assertEqual(response.status_code, 204) + + @responses.activate + def test_rpc_retries_on_failure(self): + """RPC request retries on connection error when configured.""" + self._add_login_responses() + + # Configure retry to allow POST retries (RPC uses POST) and retry on 503 + retry_config = Retry(total=3, allowed_methods=["POST"], status_forcelist=[503]) + + # First two RPC attempts return 503, third succeeds + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/switch/disconnect", + json={}, + status=503, + ) + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/switch/disconnect", + json={}, + status=503, + ) + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/switch/disconnect", + json={"result": "disconnected"}, + status=200, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + result = client.rpc("controller/core/switch/disconnect", data={}) + + # Should succeed after retries + self.assertEqual(result, {"result": "disconnected"}) + + @responses.activate + def test_schema_retries_on_failure(self): + """Schema request retries on connection error.""" + self._add_login_responses() + + # Configure retry to retry on 503 + retry_config = Retry(total=3, status_forcelist=[503]) + + # First two schema attempts return 503, third succeeds + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/schema/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/schema/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/schema/controller/core/switch", + json={"type": "object"}, + status=200, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + result = client.schema("controller/core/switch") + + # Should succeed after retries + self.assertEqual(result, {"type": "object"}) + + @responses.activate + def test_retries_exhausted_raises(self): + """After exhausting retries, exception is raised.""" + self._add_login_responses() + + # Configure retry to retry on 503 + retry_config = Retry(total=3, status_forcelist=[503], raise_on_status=False) + + # All attempts return 503 + for _ in range(10): # More than enough failures to exhaust retries + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + + # Should raise HTTPError after retries are exhausted (from raise_for_status) + with self.assertRaises(requests.exceptions.HTTPError): + client.get("controller/core/switch") + + @responses.activate + def test_retry_with_status_forcelist(self): + """Retry specific HTTP status codes when configured.""" + self._add_login_responses() + + # Configure retry to retry on 503 status + retry_config = Retry(total=3, status_forcelist=[503]) + + # First two attempts return 503, third succeeds + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json=[{"name": "switch1"}], + status=200, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + result = client.get("controller/core/switch") + + # Should succeed after retries + self.assertEqual(result, [{"name": "switch1"}]) + + @responses.activate + def test_no_retry_when_not_configured(self): + """Verify default behavior unchanged - no retries when not configured.""" + self._add_login_responses() + + # First attempt fails + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + body=ConnectionError("Connection failed"), + ) + + client = pybsn.connect(self.url, "admin", "somepassword") # No retries parameter + + # Should raise exception immediately without retries + with self.assertRaises(ConnectionError): + client.get("controller/core/switch") + + @responses.activate + def test_retry_backoff_behavior(self): + """Verify exponential backoff when using Retry object.""" + self._add_login_responses() + + # Configure retry with backoff and retry on 503 + retry_config = Retry(total=3, backoff_factor=0.1, status_forcelist=[503]) + + # First two GET attempts return 503, third succeeds + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json=[{"name": "switch1"}], + status=200, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + result = client.get("controller/core/switch") + + # Should succeed after retries + # Note: Backoff behavior is handled by urllib3 internally + self.assertEqual(result, [{"name": "switch1"}]) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_retry_bigdbclient_integration.py b/test/test_retry_bigdbclient_integration.py new file mode 100644 index 00000000..f2e6b6ec --- /dev/null +++ b/test/test_retry_bigdbclient_integration.py @@ -0,0 +1,285 @@ +import json +import sys +import unittest + +import requests +import responses +from requests.exceptions import ConnectionError +from urllib3.util.retry import Retry + +import pybsn + +sys.path.append("test") + + +class TestRetryBigDbClientIntegration(unittest.TestCase): + """ + Integration tests with actual network failures to verify retry behavior end-to-end. + """ + + url = "http://127.0.0.1:8080" + + def _login_cb(self, req): + self.assertEqual(json.loads(req.body), {"user": "admin", "password": "somepassword"}) + headers = {"Set-Cookie": "session_cookie=v_8yOI7FI-WKr-5QU6nxoJkVtAu5rKI-; Path=/api/;"} + return ( + 200, + headers, + json.dumps( + { + "success": True, + "session-cookie": "UPhNWlmDN0re8cg9xsqe9QT1QvQTznji", + "error-message": "", + } + ), + ) + + def _add_login_responses(self): + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/auth/healthy", + json={}, + status=200, + ) + responses.add_callback( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/aaa/session/login", + callback=self._login_cb, + content_type="application/json", + ) + + @responses.activate + def test_retry_succeeds_after_transient_failure(self): + """Server fails twice, succeeds third time, verify success.""" + self._add_login_responses() + + # Configure retry to retry on 503 + retry_config = Retry(total=5, status_forcelist=[503]) + + # Simulate transient failures (503) followed by success + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json=[{"name": "switch1", "mac-address": "00:00:00:00:00:01"}], + status=200, + ) + + # Connect with retry configuration + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + + # Should succeed after retries + result = client.get("controller/core/switch") + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["name"], "switch1") + + @responses.activate + def test_retry_with_connection_error(self): + """Server unreachable, verify retries happen and eventually fail.""" + self._add_login_responses() + + # Configure retry to retry on 503 + retry_config = Retry(total=3, status_forcelist=[503], raise_on_status=False) + + # All attempts fail with 503 + for _ in range(10): + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={}, + status=503, + ) + + # Connect with limited retry configuration + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + + # Should raise HTTPError after exhausting retries + with self.assertRaises(requests.exceptions.HTTPError): + client.get("controller/core/switch") + + @responses.activate + def test_retry_respects_status_forcelist(self): + """Only retry specific HTTP status codes.""" + self._add_login_responses() + + # Configure to retry only on 503 Service Unavailable + retry_config = Retry(total=3, status_forcelist=[503]) + + # First attempt returns 503, second succeeds + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={"error": "Service Unavailable"}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json=[{"name": "switch1"}], + status=200, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + + # Should succeed after retrying 503 + result = client.get("controller/core/switch") + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["name"], "switch1") + + @responses.activate + def test_retry_does_not_retry_non_forcelist_status(self): + """Do not retry status codes not in forcelist.""" + self._add_login_responses() + + # Configure to retry only on 503 + retry_config = Retry(total=3, status_forcelist=[503], raise_on_status=False) + + # Return 500 (not in forcelist), should not retry + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={"error": "Internal Server Error"}, + status=500, + ) + + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + + # Should fail without retries because 500 is not in forcelist + # But since raise_on_status=False, it returns the response + with self.assertRaises(Exception): # HTTPError from raise_for_status + client.get("controller/core/switch") + + @responses.activate + def test_no_retry_when_not_configured(self): + """Verify default behavior unchanged - no retries when not configured.""" + self._add_login_responses() + + # First attempt fails + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + body=ConnectionError("Connection failed"), + ) + + # Connect without retry configuration + client = pybsn.connect(self.url, "admin", "somepassword") + + # Should fail immediately without retries + with self.assertRaises(ConnectionError): + client.get("controller/core/switch") + + @responses.activate + def test_retry_on_guess_url(self): + """Verify retries work during URL guessing phase with 503 status.""" + # Configure retry to retry on 503 + retry_config = Retry(total=3, status_forcelist=[503]) + + # First attempt to health check returns 503, second succeeds + responses.add( + responses.GET, + "https://127.0.0.1:8443/api/v1/auth/healthy", + json={}, + status=503, + ) + responses.add( + responses.GET, + "https://127.0.0.1:8443/api/v1/auth/healthy", + json={}, + status=200, + ) + + # Login response + responses.add_callback( + responses.POST, + "https://127.0.0.1:8443/api/v1/rpc/controller/core/aaa/session/login", + callback=self._login_cb, + content_type="application/json", + ) + + # Connect with retry configuration (host without scheme) + client = pybsn.connect("127.0.0.1", "admin", "somepassword", retries=retry_config) + + # Should succeed after retry during URL guessing + self.assertIsNotNone(client) + self.assertEqual(client.url, "https://127.0.0.1:8443") + + @responses.activate + def test_retry_during_login(self): + """Verify retries work during login phase with 503 status.""" + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/auth/healthy", + json={}, + status=200, + ) + + # Configure to retry POST (login uses POST) on 503 + retry_config = Retry(total=3, allowed_methods=["POST"], status_forcelist=[503]) + + # First login attempt returns 503, second succeeds + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/aaa/session/login", + json={}, + status=503, + ) + responses.add_callback( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/aaa/session/login", + callback=self._login_cb, + content_type="application/json", + ) + + # Connect with retry configuration + client = pybsn.connect(self.url, "admin", "somepassword", retries=retry_config) + + # Should succeed after retry during login + self.assertIsNotNone(client) + + @responses.activate + def test_retry_with_token_validation(self): + """Verify retries work during token validation with 503 status.""" + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/auth/healthy", + json={}, + status=200, + ) + + # Configure retry to retry on 503 + retry_config = Retry(total=3, status_forcelist=[503]) + + # First token validation returns 503, second succeeds + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/aaa/auth-context", + json={}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/aaa/auth-context", + json=[{"user-info": {"user-name": "admin"}}], + status=200, + ) + + # Connect with token and retry configuration + client = pybsn.connect(self.url, token="test-token", retries=retry_config) + + # Should succeed after retry during token validation + self.assertIsNotNone(client) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_retry_connect.py b/test/test_retry_connect.py new file mode 100644 index 00000000..134d2d7b --- /dev/null +++ b/test/test_retry_connect.py @@ -0,0 +1,117 @@ +import sys +import unittest +from unittest.mock import MagicMock, patch + +import requests +from urllib3.util.retry import Retry + +import pybsn + +sys.path.append("test") + + +class TestRetryConnect(unittest.TestCase): + """ + Test that the retry parameter is properly configured during connection setup. + """ + + def test_connect_no_retries(self): + """Default behavior (retries=None), verify no custom HTTPAdapter mounted.""" + with patch.object(requests.Session, "send") as mock_send: + mock_send.return_value = self._create_mock_response(200, {"session-cookie": "test-cookie"}) + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password") + # Verify session has default adapters (not custom retry adapters) + # Default sessions have basic HTTPAdapters with max_retries=0 (Retry(0)) + adapter = client.session.get_adapter("http://example.com") + self.assertIsInstance(adapter, requests.adapters.HTTPAdapter) + # Default adapter has max_retries=0 or Retry(0, read=False) + if isinstance(adapter.max_retries, int): + self.assertEqual(adapter.max_retries, 0) + else: + # Default Retry object has total=0 + self.assertEqual(adapter.max_retries.total, 0) + + def test_connect_retries_int(self): + """Pass retries=3, verify HTTPAdapter mounted with correct int value.""" + with patch.object(requests.Session, "send") as mock_send: + mock_send.return_value = self._create_mock_response(200, {"session-cookie": "test-cookie"}) + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password", retries=3) + + # Verify adapters have max_retries configured with total=3 + # Note: int gets converted to Retry object by urllib3 + http_adapter = client.session.get_adapter("http://example.com") + https_adapter = client.session.get_adapter("https://example.com") + + self.assertIsInstance(http_adapter, requests.adapters.HTTPAdapter) + self.assertIsInstance(https_adapter, requests.adapters.HTTPAdapter) + self.assertIsInstance(http_adapter.max_retries, Retry) + self.assertIsInstance(https_adapter.max_retries, Retry) + self.assertEqual(http_adapter.max_retries.total, 3) + self.assertEqual(https_adapter.max_retries.total, 3) + + def test_connect_retries_retry_object(self): + """Pass Retry(total=5, backoff_factor=0.5), verify correct object.""" + retry_config = Retry(total=5, backoff_factor=0.5, status_forcelist=[500, 502, 503, 504]) + + with patch.object(requests.Session, "send") as mock_send: + mock_send.return_value = self._create_mock_response(200, {"session-cookie": "test-cookie"}) + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password", retries=retry_config) + + # Verify adapters have the correct Retry object + http_adapter = client.session.get_adapter("http://example.com") + https_adapter = client.session.get_adapter("https://example.com") + + self.assertIsInstance(http_adapter, requests.adapters.HTTPAdapter) + self.assertIsInstance(https_adapter, requests.adapters.HTTPAdapter) + self.assertEqual(http_adapter.max_retries, retry_config) + self.assertEqual(https_adapter.max_retries, retry_config) + + def test_connect_retries_with_token(self): + """Retries work with token auth.""" + with patch.object(requests.Session, "send") as mock_send: + mock_send.return_value = self._create_mock_response(200, []) + client = pybsn.connect("http://127.0.0.1:8080", token="test-token", retries=3) + + # Verify adapters have max_retries configured with total=3 + # Note: int gets converted to Retry object by urllib3 + http_adapter = client.session.get_adapter("http://example.com") + https_adapter = client.session.get_adapter("https://example.com") + + self.assertIsInstance(http_adapter, requests.adapters.HTTPAdapter) + self.assertIsInstance(https_adapter, requests.adapters.HTTPAdapter) + self.assertIsInstance(http_adapter.max_retries, Retry) + self.assertIsInstance(https_adapter.max_retries, Retry) + self.assertEqual(http_adapter.max_retries.total, 3) + self.assertEqual(https_adapter.max_retries.total, 3) + + def test_connect_retries_preserves_verify_tls_true(self): + """Verify TLS verification still works when verify_tls=True and retries set.""" + with patch.object(requests.Session, "mount"): + with patch.object(requests.Session, "send") as mock_send: + mock_send.return_value = self._create_mock_response(200, {"session-cookie": "test-cookie"}) + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password", verify_tls=True, retries=3) + + # Verify TLS verification is enabled + self.assertTrue(client.session.verify) + + def test_connect_retries_preserves_verify_tls_false(self): + """Verify TLS verification disabled when verify_tls=False and retries set.""" + with patch.object(requests.Session, "mount"): + with patch.object(requests.Session, "send") as mock_send: + mock_send.return_value = self._create_mock_response(200, {"session-cookie": "test-cookie"}) + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password", verify_tls=False, retries=3) + + # Verify TLS verification is disabled + self.assertFalse(client.session.verify) + + def _create_mock_response(self, status_code, json_data): + """Helper to create a mock response object.""" + response = MagicMock(spec=requests.Response) + response.status_code = status_code + response.json.return_value = json_data + response.raise_for_status.return_value = None + return response + + +if __name__ == "__main__": + unittest.main() From 8d33f26a6b82747288a44b9aea84e5bef32f8f5a Mon Sep 17 00:00:00 2001 From: Andreas Wundsam Date: Mon, 13 Oct 2025 18:03:06 +0200 Subject: [PATCH 2/2] Clarify retry behavior and add comprehensive validation tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The initial documentation was misleading about what retries=3 does. This commit fixes the documentation and adds validation tests. Key clarifications: - retries=3 only retries GET, HEAD, OPTIONS, PUT, DELETE, TRACE - POST and PATCH are NOT retried even on connection errors - Integer values do NOT retry on HTTP status codes (only connection errors) - allowed_methods restricts retries for ALL failure types, not just status codes Changes: - Updated docstring with precise behavior description - Updated README with accurate documentation and warnings - Updated example comments to match tested behavior - Added test_retry_behavior_validation.py (6 tests) - validates configuration - Added test_retry_real_server.py (3 tests) - real HTTP server integration tests All 118 tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- README.md | 12 +- examples/retry_example.py | 7 +- pybsn/__init__.py | 17 +- test/test_retry_behavior_validation.py | 216 +++++++++++++++++++++++++ test/test_retry_real_server.py | 190 ++++++++++++++++++++++ 5 files changed, 430 insertions(+), 12 deletions(-) create mode 100644 test/test_retry_behavior_validation.py create mode 100644 test/test_retry_real_server.py diff --git a/README.md b/README.md index d6532d2f..0cd830e3 100644 --- a/README.md +++ b/README.md @@ -184,14 +184,20 @@ pybsn supports automatic retry of failed HTTP requests to improve reliability wh ### Simple Retry Count -Pass an integer to specify the total number of retry attempts: +Pass an integer to specify retry attempts for **connection-level failures only**: ```python -# Retry up to 3 times on failures +# Retry up to 3 times on connection errors, timeouts, DNS failures client = pybsn.connect(host="controller", token="", retries=3) ``` -By default, only idempotent HTTP methods (GET, HEAD, OPTIONS, TRACE) are automatically retried. Non-idempotent methods (POST, PUT, PATCH, DELETE) are not retried to prevent unintended side effects. +**Important:** When using an integer value: +- Retries **GET, HEAD, OPTIONS, PUT, DELETE, TRACE** requests (but **NOT POST or PATCH**) +- Retries only on **connection-level failures** (connection errors, timeouts, DNS failures) +- Does **NOT** retry on HTTP error status codes like 503 or 504 +- No exponential backoff (immediate retry) + +To retry on HTTP status codes or include POST/PATCH requests, use a `Retry` object (see below). ### Advanced Retry Configuration diff --git a/examples/retry_example.py b/examples/retry_example.py index cce8ed50..7924e0ff 100644 --- a/examples/retry_example.py +++ b/examples/retry_example.py @@ -20,12 +20,13 @@ def example_simple_retry(): - """Example 1: Simple integer retry count (retries GET requests 3 times)""" + """Example 1: Simple integer retry count (retries on connection-level failures only)""" print("Example 1: Simple integer retry count") client = pybsn.connect(args.host, args.user, args.password, retries=3) - # This will retry up to 3 times on connection errors or certain HTTP status codes - # By default, only idempotent methods (GET, HEAD, OPTIONS, TRACE) are retried + # This will retry up to 3 times on connection errors, timeouts, DNS failures + # Retries GET, HEAD, OPTIONS, PUT, DELETE, TRACE (but NOT POST or PATCH) + # Does NOT retry on HTTP status codes like 503 - only connection-level failures switches = client.root.core.switch() print(f" Found {len(switches)} switches") diff --git a/pybsn/__init__.py b/pybsn/__init__.py index a4e4c6c9..e74d4849 100644 --- a/pybsn/__init__.py +++ b/pybsn/__init__.py @@ -649,12 +649,17 @@ def connect( for future operations unless it is changed. :parameter retries: Optional retry configuration for failed HTTP requests. - None (default) - No automatic retries, preserving existing behavior. - int - Total number of retry attempts (e.g., retries=3). - urllib3.util.retry.Retry - Full retry configuration object for advanced control - (e.g., retries=Retry(total=5, backoff_factor=1, status_forcelist=[500, 502, 503, 504])). - Note: By default, only idempotent methods (GET, HEAD, OPTIONS, TRACE) are retried. - Non-idempotent methods (POST, PUT, PATCH, DELETE) require explicit configuration. + None (default) - No automatic retries. + int - Retry count for connection-level failures only (e.g., retries=3). + When an integer is specified, retries GET, HEAD, OPTIONS, PUT, DELETE, TRACE requests + on connection errors, timeouts, and DNS failures. + Does NOT retry on HTTP error status codes (e.g., 503, 504). + Does NOT retry POST or PATCH requests. + No exponential backoff (immediate retry). + urllib3.util.retry.Retry - Full retry configuration object for advanced control. + Use this to retry on HTTP status codes, configure backoff, or retry POST/PATCH. + Example: retries=Retry(total=5, backoff_factor=1, status_forcelist=[503, 504], + allowed_methods=["GET", "POST"]). (other parameters for advanced/internal use). diff --git a/test/test_retry_behavior_validation.py b/test/test_retry_behavior_validation.py new file mode 100644 index 00000000..6128e754 --- /dev/null +++ b/test/test_retry_behavior_validation.py @@ -0,0 +1,216 @@ +#!/usr/bin/env python +""" +Test to validate and document the exact behavior of retries parameter. + +This test documents what actually happens when you pass retries=3 vs retries=Retry(...) +""" +import unittest + +import requests +import responses +from urllib3.util.retry import Retry + +import pybsn + + +class TestRetryBehaviorValidation(unittest.TestCase): + """Validate exact retry behavior with different configurations.""" + + @responses.activate + def test_retry_int_creates_retry_object(self): + """Verify that retries=3 creates a Retry object with specific defaults.""" + responses.add(responses.GET, "http://127.0.0.1:8080/api/v1/auth/healthy", json={}, status=200) + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/aaa/session/login", + json={"session-cookie": "test"}, + status=200, + ) + + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password", retries=3) + + adapter = client.session.get_adapter("http://example.com") + retry_config = adapter.max_retries + + # When retries=3, urllib3.Retry.from_int(3) is called, which creates: + self.assertIsInstance(retry_config, Retry) + self.assertEqual(retry_config.total, 3, "Should retry up to 3 times") + + # Methods that will be retried (idempotent + PUT/DELETE) + self.assertEqual( + retry_config.allowed_methods, + frozenset({"GET", "HEAD", "OPTIONS", "PUT", "DELETE", "TRACE"}), + "Should retry idempotent methods plus PUT/DELETE, but NOT POST or PATCH", + ) + + # Status codes - IMPORTANT: empty by default! + self.assertEqual( + retry_config.status_forcelist, + set(), + "Should NOT retry any HTTP status codes by default (only connection errors)", + ) + + # Backoff + self.assertEqual(retry_config.backoff_factor, 0, "Should have no exponential backoff by default") + + @responses.activate + def test_retry_int_does_not_retry_http_errors(self): + """Verify that retries=3 does NOT retry HTTP error status codes like 503.""" + responses.add(responses.GET, "http://127.0.0.1:8080/api/v1/auth/healthy", json={}, status=200) + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/aaa/session/login", + json={"session-cookie": "test"}, + status=200, + ) + + # Return 503 error + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={"error": "Service Unavailable"}, + status=503, + ) + + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password", retries=3) + + # Should NOT retry 503 - it will raise immediately + with self.assertRaises(requests.exceptions.HTTPError) as cm: + client.get("controller/core/switch") + + self.assertEqual(cm.exception.response.status_code, 503) + # Verify only 1 request was made (no retries on status codes) + get_requests = [call for call in responses.calls if call.request.method == "GET" and "switch" in call.request.url] + self.assertEqual(len(get_requests), 1, "Should only make 1 request (no retry on HTTP status codes)") + + @responses.activate + def test_retry_custom_object_retries_status_codes(self): + """Verify that Retry object with status_forcelist DOES retry HTTP errors.""" + responses.add(responses.GET, "http://127.0.0.1:8080/api/v1/auth/healthy", json={}, status=200) + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/aaa/session/login", + json={"session-cookie": "test"}, + status=200, + ) + + # First request returns 503, second succeeds + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json={"error": "Service Unavailable"}, + status=503, + ) + responses.add( + responses.GET, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + json=[{"name": "switch1"}], + status=200, + ) + + # Use Retry object with status_forcelist + retry_config = Retry(total=3, status_forcelist=[503]) + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password", retries=retry_config) + + # Should succeed after retry + result = client.get("controller/core/switch") + self.assertEqual(result, [{"name": "switch1"}]) + + # Verify 2 requests were made + get_requests = [call for call in responses.calls if call.request.method == "GET" and "switch" in call.request.url] + self.assertEqual(len(get_requests), 2, "Should make 2 requests (1 failure + 1 retry)") + + @responses.activate + def test_retry_int_does_not_retry_post_on_connection_error(self): + """Verify that retries=3 does NOT retry POST even on connection errors.""" + responses.add(responses.GET, "http://127.0.0.1:8080/api/v1/auth/healthy", json={}, status=200) + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/aaa/session/login", + json={"session-cookie": "test"}, + status=200, + ) + + # Simulate connection error on POST + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + body=requests.exceptions.ConnectionError("Connection failed"), + ) + + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password", retries=3) + + # Should NOT retry POST - will raise immediately even on connection error + with self.assertRaises(requests.exceptions.ConnectionError): + client.post("controller/core/switch", data={"name": "test"}) + + # Verify only 1 POST request was made (no retries for POST with retries=int) + post_requests = [ + call for call in responses.calls if call.request.method == "POST" and "switch" in call.request.url + ] + self.assertEqual(len(post_requests), 1, "POST should NOT be retried even on connection errors") + + @responses.activate + def test_retry_int_does_not_retry_patch_on_connection_error(self): + """Verify that retries=3 does NOT retry PATCH even on connection errors.""" + responses.add(responses.GET, "http://127.0.0.1:8080/api/v1/auth/healthy", json={}, status=200) + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/aaa/session/login", + json={"session-cookie": "test"}, + status=200, + ) + + # Simulate connection error on PATCH + responses.add( + responses.PATCH, + "http://127.0.0.1:8080/api/v1/data/controller/core/switch", + body=requests.exceptions.ConnectionError("Connection failed"), + ) + + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password", retries=3) + + # Should NOT retry PATCH - will raise immediately even on connection error + with self.assertRaises(requests.exceptions.ConnectionError): + client.patch("controller/core/switch", data={"name": "test"}) + + # Verify only 1 PATCH request was made + patch_requests = [ + call for call in responses.calls if call.request.method == "PATCH" and "switch" in call.request.url + ] + self.assertEqual(len(patch_requests), 1, "PATCH should NOT be retried even on connection errors") + + @responses.activate + def test_retry_int_allowed_methods_config(self): + """ + Verify that retries=3 configures allowed_methods correctly. + + Note: We test configuration rather than actual retry behavior on connection errors + because the 'responses' mocking library interferes with urllib3's retry mechanism. + The responses library raises exceptions at the wrong layer, preventing retries. + + The configuration test proves that GET/PUT/DELETE will be retried while POST/PATCH won't. + """ + responses.add(responses.GET, "http://127.0.0.1:8080/api/v1/auth/healthy", json={}, status=200) + responses.add( + responses.POST, + "http://127.0.0.1:8080/api/v1/rpc/controller/core/aaa/session/login", + json={"session-cookie": "test"}, + status=200, + ) + + client = pybsn.connect("http://127.0.0.1:8080", "admin", "password", retries=3) + + adapter = client.session.get_adapter("http://example.com") + retry_config = adapter.max_retries + + # Verify allowed_methods includes GET but not POST/PATCH + self.assertIn("GET", retry_config.allowed_methods, "GET should be in allowed_methods") + self.assertIn("PUT", retry_config.allowed_methods, "PUT should be in allowed_methods") + self.assertIn("DELETE", retry_config.allowed_methods, "DELETE should be in allowed_methods") + self.assertNotIn("POST", retry_config.allowed_methods, "POST should NOT be in allowed_methods") + self.assertNotIn("PATCH", retry_config.allowed_methods, "PATCH should NOT be in allowed_methods") + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_retry_real_server.py b/test/test_retry_real_server.py new file mode 100644 index 00000000..546c9379 --- /dev/null +++ b/test/test_retry_real_server.py @@ -0,0 +1,190 @@ +#!/usr/bin/env python +""" +Real integration test with actual HTTP server to validate retry behavior. + +This test uses a real HTTP server on an ephemeral port to verify that: +1. GET requests ARE retried on connection failures +2. POST requests are NOT retried on connection failures (even with retries=3) +3. PATCH requests are NOT retried on connection failures (even with retries=3) +""" +import http.server +import json +import socketserver +import threading +import time +import unittest + +import requests + +import pybsn + + +class RequestCountingHandler(http.server.BaseHTTPRequestHandler): + """HTTP handler that tracks request counts""" + + # Class variables to track requests + get_count = 0 + post_count = 0 + patch_count = 0 + fail_first_n_gets = 0 # Fail first N GET requests by closing connection + + @classmethod + def reset_counts(cls): + cls.get_count = 0 + cls.post_count = 0 + cls.patch_count = 0 + cls.fail_first_n_gets = 0 + + def log_message(self, format, *args): + """Suppress server logs during tests""" + pass + + def do_GET(self): + RequestCountingHandler.get_count += 1 + + # Fail first N GETs by closing connection (simulates connection error) + if RequestCountingHandler.get_count <= RequestCountingHandler.fail_first_n_gets: + # Close connection without sending response + self.wfile.close() + return + + # Successful response + if "/api/v1/auth/healthy" in self.path: + self.send_response(200) + self.send_header("Content-type", "application/json") + self.end_headers() + self.wfile.write(b"{}") + elif "/api/v1/data/controller/core/switch" in self.path: + result = [{"name": "switch1"}] + response = json.dumps(result).encode() + self.send_response(200) + self.send_header("Content-type", "application/json") + self.send_header("Content-length", str(len(response))) + self.end_headers() + self.wfile.write(response) + + def do_POST(self): + RequestCountingHandler.post_count += 1 + + # Close connection immediately (simulate connection error) + # This tests that POST is NOT retried + if "/api/v1/data/controller/core/switch" in self.path: + self.wfile.close() + return + + # Login endpoint + if "/api/v1/rpc/controller/core/aaa/session/login" in self.path: + result = { + "success": True, + "session-cookie": "test-cookie", + "error-message": "", + } + response = json.dumps(result).encode() + self.send_response(200) + self.send_header("Content-type", "application/json") + self.send_header("Content-length", str(len(response))) + self.end_headers() + self.wfile.write(response) + + def do_PATCH(self): + RequestCountingHandler.patch_count += 1 + + # Close connection immediately (simulate connection error) + # This tests that PATCH is NOT retried + self.wfile.close() + + +class TestRetryRealServer(unittest.TestCase): + """Test retry behavior with real HTTP server""" + + @classmethod + def setUpClass(cls): + """Start HTTP server on ephemeral port""" + # Use port 0 for automatic ephemeral port assignment + cls.server = socketserver.TCPServer(("127.0.0.1", 0), RequestCountingHandler) + cls.port = cls.server.server_address[1] + cls.url = f"http://127.0.0.1:{cls.port}" + + # Start server in background thread + cls.server_thread = threading.Thread(target=cls.server.serve_forever, daemon=True) + cls.server_thread.start() + + # Give server time to start + time.sleep(0.1) + + @classmethod + def tearDownClass(cls): + """Stop HTTP server""" + cls.server.shutdown() + cls.server.server_close() + + def setUp(self): + """Reset request counts before each test""" + RequestCountingHandler.reset_counts() + + def test_retry_int_does_retry_get_on_real_connection_error(self): + """ + Real test: GET requests ARE retried on connection errors with retries=3. + + This uses a real HTTP server that drops connections to verify urllib3 retry behavior. + """ + # Configure server to fail first 2 GETs by closing connection + RequestCountingHandler.fail_first_n_gets = 2 + + # Connect with retries=3 + client = pybsn.connect(self.url, "admin", "password", retries=3) + + # This should succeed after 2 retries (3rd attempt succeeds) + result = client.get("controller/core/switch") + + # Verify we got the result + self.assertEqual(result, [{"name": "switch1"}]) + + # Verify 3 GET requests were made to /switch (2 failures + 1 success) + # Note: Additional GETs to /healthy during connection setup + self.assertGreaterEqual( + RequestCountingHandler.get_count, + 3, + "Should have made at least 3 GET requests (2 failed + 1 success)", + ) + + def test_retry_int_does_not_retry_post_on_real_connection_error(self): + """ + Real test: POST requests are NOT retried on connection errors with retries=3. + + This uses a real HTTP server that drops POST connections to verify urllib3 does NOT retry. + """ + # Connect with retries=3 + client = pybsn.connect(self.url, "admin", "password", retries=3) + + # POST that gets connection closed should NOT be retried + with self.assertRaises(Exception): # Connection error or similar + client.post("controller/core/switch", data={"name": "test"}) + + # Verify only 1 POST request was made (no retries) + # Note: +1 for login POST + self.assertEqual( + RequestCountingHandler.post_count, + 2, # 1 login + 1 failed POST + "Should only make 1 POST request to /switch (no retries)", + ) + + def test_retry_int_does_not_retry_patch_on_real_connection_error(self): + """ + Real test: PATCH requests are NOT retried on connection errors with retries=3. + + This uses a real HTTP server that drops PATCH connections to verify urllib3 does NOT retry. + """ + # Connect with retries=3 + client = pybsn.connect(self.url, "admin", "password", retries=3) + + # PATCH that gets connection closed should NOT be retried + with self.assertRaises(Exception): # Connection error or similar + client.patch("controller/core/switch", data={"name": "test"}) + + # Verify only 1 PATCH request was made (no retries) + self.assertEqual(RequestCountingHandler.patch_count, 1, "Should only make 1 PATCH request (no retries)") + + +if __name__ == "__main__": + unittest.main()