danielcweeks commented on code in PR #3418:
URL: https://github.com/apache/iceberg-python/pull/3418#discussion_r3461677597
##########
tests/catalog/test_rest.py:
##########
@@ -2287,6 +2025,139 @@ def test_request_session_with_ssl_client_cert() -> None:
assert "Could not find the TLS certificate file, invalid path:
path_to_client_cert" in str(e.value)
+def test_session_without_connection_config_uses_default_adapter(rest_mock:
Mocker) -> None:
+ catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN)
+ for adapter in catalog._session.adapters.values():
+ assert not isinstance(adapter, _RetryTimeoutHTTPAdapter)
+
+
+def test_session_with_connection_timeout_and_retries(rest_mock: Mocker) ->
None:
+ catalog_properties = {
+ "uri": TEST_URI,
+ "token": TEST_TOKEN,
+ CONNECTION: {
+ CONNECTION_TIMEOUT: 60,
+ CONNECTION_RETRIES: 5,
+ CONNECTION_BACKOFF_FACTOR: 1.0,
+ },
+ }
+ catalog = RestCatalog("rest", **catalog_properties) # type: ignore
+
+ https_adapter = catalog._session.adapters["https://";]
+ http_adapter = catalog._session.adapters["http://";]
+ assert isinstance(https_adapter, _RetryTimeoutHTTPAdapter)
+ assert https_adapter is http_adapter
+ assert https_adapter._timeout == 60.0
+ assert https_adapter.max_retries.total == 5
+ assert https_adapter.max_retries.backoff_factor == 1.0
+ # Internal retry policy: transient codes and idempotent methods only.
+ assert https_adapter.max_retries.status_forcelist == [429, 500, 502, 503,
504]
+ allowed_methods = https_adapter.max_retries.allowed_methods or frozenset()
+ assert set(allowed_methods) == {"GET", "HEAD", "OPTIONS"}
+
+
+def test_session_with_connection_timeout_only(rest_mock: Mocker) -> None:
+ catalog_properties = {
+ "uri": TEST_URI,
+ "token": TEST_TOKEN,
+ CONNECTION: {CONNECTION_TIMEOUT: "30"},
+ }
+ catalog = RestCatalog("rest", **catalog_properties) # type: ignore
+ adapter = catalog._session.adapters["https://";]
+ assert isinstance(adapter, _RetryTimeoutHTTPAdapter)
+ assert adapter._timeout == 30.0
+ # Default retry policy (total=0) is a no-op when only timeout is
configured.
+ assert adapter.max_retries.total == 0
+
+
+@contextmanager
+def _local_rest_server_503_then_200(num_failures: int) -> Iterator[dict[str,
Any]]:
+ """Stand up a loopback HTTP server that returns `num_failures` 503s for
`/v1/namespaces` then a 200.
+
+ Used in place of `requests_mock`, which replaces the HTTPAdapter and would
bypass the retry logic.
+
+ Yields a dict with `port` and `namespace_calls` keys (the latter is
updated in-place as requests arrive).
+ """
+ import json
+ import threading
+ from http.server import BaseHTTPRequestHandler, HTTPServer
+
+ state: dict[str, Any] = {"namespace_calls": 0}
+ config_body = json.dumps(
+ {"defaults": {}, "overrides": {}, "endpoints": [str(endpoint) for
endpoint in TEST_SUPPORTED_ENDPOINTS]}
+ ).encode()
+
+ class _Handler(BaseHTTPRequestHandler):
+ def do_GET(self) -> None:
+ if self.path.endswith("/v1/config"):
+ self._respond(200, config_body)
+ elif self.path.endswith("/v1/namespaces"):
+ state["namespace_calls"] += 1
+ if state["namespace_calls"] <= num_failures:
+ self._respond(503, b"")
+ else:
+ self._respond(200, json.dumps({"namespaces":
[["foo"]]}).encode())
+ else:
+ self._respond(404, b"")
+
+ def _respond(self, status: int, body: bytes) -> None:
+ self.send_response(status)
+ self.send_header("Content-Type", "application/json")
+ self.send_header("Content-Length", str(len(body)))
+ self.end_headers()
+ if body:
+ self.wfile.write(body)
+
+ def log_message(self, format: str, *args: Any) -> None: # silence
default access logs
+ pass
+
+ server = HTTPServer(("127.0.0.1", 0), _Handler)
+ state["port"] = server.server_address[1]
+ server_thread = threading.Thread(target=server.serve_forever, daemon=True)
+ server_thread.start()
+ try:
+ yield state
+ finally:
+ server.shutdown()
+ server.server_close()
+
+
+def test_session_retries_on_transient_5xx_then_succeeds() -> None:
Review Comment:
We should add a test that throws `5XX` enough times for the retries to be
exhausted, which would surface the `RetryError` issue above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
