Fokko commented on code in PR #3418:
URL: https://github.com/apache/iceberg-python/pull/3418#discussion_r3947504256
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -442,6 +453,111 @@ class ListViewsResponse(IcebergBaseModel):
_PLANNING_RESPONSE_ADAPTER = TypeAdapter(PlanningResponse)
+_T = TypeVar("_T", int, float)
+
+
+def _parse_connection_property(
+ properties: Properties,
+ property_name: str,
+ converter: Callable[[Any], _T],
+ type_description: str,
+ is_invalid: Callable[[_T], bool],
+ range_description: str,
+) -> _T | None:
+ raw_value = properties.get(property_name)
+ if raw_value is None:
+ return None
+ try:
+ value = converter(raw_value)
+ except (TypeError, ValueError) as e:
+ raise ValueError(f"`{property_name}` must be {type_description}, got:
{raw_value!r}") from e
+ if is_invalid(value):
+ raise ValueError(f"`{property_name}` must be {range_description}, got:
{value}")
+ return value
+
+
+class _RetryTimeoutHTTPAdapter(HTTPAdapter):
+ """HTTPAdapter that applies a default per-request timeout.
+
+ requests does not provide a way to set a default timeout on a Session;
+ without this adapter, every call would have to thread `timeout=` through.
+ The adapter applies `self._timeout` whenever a per-call timeout is not set.
+ """
+
+ def __init__(self, timeout: float | None = None, max_retries: Retry | int
= DEFAULT_RETRIES) -> None:
+ self._timeout = timeout
+ super().__init__(max_retries=max_retries)
+
+ def send(
+ self,
+ request: PreparedRequest,
+ stream: bool = False,
+ timeout: None | float | tuple[float, float] | tuple[float, None] =
None,
+ verify: bool | str = True,
+ cert: None | bytes | str | tuple[bytes | str, bytes | str] = None,
+ proxies: Mapping[str, str] | None = None,
+ ) -> Response:
+ if timeout is None:
+ timeout = self._timeout
+ return super().send(request, stream=stream, timeout=timeout,
verify=verify, cert=cert, proxies=proxies)
+
+
+def _create_connection_adapter(properties: Properties) ->
_RetryTimeoutHTTPAdapter | None:
+ """Build a connection adapter from the optional `rest.client.*` properties.
+
+ Returns None when no connection properties are supplied, leaving the
default
+ Session behavior unchanged. Raises ValueError on invalid input.
+ """
+ if not any(
+ property_name in properties
+ for property_name in (REST_CLIENT_REQUEST_TIMEOUT,
REST_CLIENT_MAX_RETRIES, REST_CLIENT_RETRY_BACKOFF_FACTOR)
+ ):
+ return None
+
+ timeout = _parse_connection_property(
+ properties,
+ REST_CLIENT_REQUEST_TIMEOUT,
+ float,
+ "a number",
+ lambda value: value <= 0,
+ "a positive number",
+ )
+
+ retries = _parse_connection_property(
+ properties,
+ REST_CLIENT_MAX_RETRIES,
+ int,
+ "an integer",
+ lambda value: value < 0,
+ "non-negative",
+ )
+ backoff_factor = _parse_connection_property(
+ properties,
+ REST_CLIENT_RETRY_BACKOFF_FACTOR,
+ float,
+ "a number",
+ lambda value: value < 0,
+ "non-negative",
+ )
Review Comment:
We can compact this into by reusing the methods in `properties.py`:
```suggestion
timeout = property_as_float(properties, REST_CLIENT_REQUEST_TIMEOUT)
if timeout is not None and timeout <= 0:
raise ValueError(f"`{REST_CLIENT_REQUEST_TIMEOUT}` must be a
positive number, got: {timeout}")
retries = property_as_int(properties, REST_CLIENT_MAX_RETRIES)
if retries is not None and retries < 0:
raise ValueError(f"`{REST_CLIENT_MAX_RETRIES}` must be non-negative,
got: {retries}")
backoff_factor = property_as_float(properties,
REST_CLIENT_RETRY_BACKOFF_FACTOR)
if backoff_factor is not None and backoff_factor < 0:
raise ValueError(f"`{REST_CLIENT_RETRY_BACKOFF_FACTOR}` must be
non-negative, got: {backoff_factor}")
```
--
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]
