David-Banquet opened a new pull request, #4099: URL: https://github.com/apache/iceberg-python/pull/4099
Closes #4076 # Rationale for this change `rest.client.connection-timeout-ms` and `rest.client.socket-timeout-ms` are ignored when `rest.sigv4-enabled` is set. `SigV4Adapter` is mounted on the catalog URI, so `requests` picks it over the timeout adapter mounted on `http://` and `https://`, and it never sets a timeout. `SigV4Adapter` now extends `_RetryTimeoutHTTPAdapter` and gets the same timeout, computed by a small `_connection_timeout` helper shared with `_create_connection_adapter`. Its retries still come from `rest.sigv4.max-retries`. # Are these changes tested? Yes. `test_sigv4_adapter_applies_connection_timeout` checks the adapter on the catalog URI carries the summed timeout and the SigV4 retry default. It fails without the change. With the repro from the issue (server answering after 8 s, 1 s + 1 s timeout): ``` without SigV4: ConnectionError after 2.0 s with SigV4: ConnectionError after 22.1 s # was: response received after 8.1 s ``` The 22 s are 11 attempts of 2 s: `SigV4Adapter` retries 10 times by default with no backoff. That is the retry behavior tracked in #3008, left unchanged here. # Are there any user-facing changes? Yes: the `rest.client.*` timeouts now apply to SigV4 catalogs (S3 Tables, Glue REST). Those properties are not released yet (#3418), so no released behavior changes. #3120 replaces `SigV4Adapter` with a plain `HTTPAdapter`; if it lands first I'll rebase this onto it. -- 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]
