TheR1sing3un opened a new pull request, #7732:
URL: https://github.com/apache/paimon/pull/7732
## Purpose
The REST `HttpClient` currently hardcodes its retry count and ignores its
own session timeout. Two practical issues fall out of this:
1. **Timeout is silently dropped.** `requests.Session.timeout` is not
consulted by the library — only `session.request(timeout=...)` is. The previous
`self.session.timeout = (180, 180)` had no effect, so requests could hang
indefinitely on a slow server.
2. **Retry counts are not tunable.** Hardcoded `max_retries=3` mixed connect
/ read retries; users could not disable connect retries (which often shouldn't
retry) or boost read retries against flaky upstreams.
This PR introduces five `CatalogOptions` so REST behaviour can be tuned via
standard catalog options:
| Key | Type | Default | Description |
|-----|------|---------|-------------|
| `http.connect-timeout` | int (sec) | 180 | TCP connect timeout |
| `http.read-timeout` | int (sec) | 180 | Response read timeout |
| `http.max-connect-retries` | int | 3 | Retries for connect errors |
| `http.max-read-retries` | int | 3 | Retries for read / status errors
(429/502/503/504) |
| `http.keep-alive` | bool | true | When false, sends `Connection: close` |
`HttpClient` now accepts an optional `Options` argument, applies the timeout
via `session.request(timeout=...)`, separates `connect` / `read` retry counters
in `ExponentialRetry` (with `total=None` so each type governs independently),
and sets `Connection: close` when keep-alive is disabled. `RESTApi` forwards
its options through. `token_loader.py` is updated to the new
`ExponentialRetry(connect_retries, read_retries)` signature.
## Linked issue
N/A — discovered while tuning REST clients against high-latency catalog
servers and verifying that `session.timeout` is dead code.
## Tests
* `pypaimon/tests/rest/test_exponential_retry_strategy.py` — refreshed for
the new signature; covers `total=None`, separated connect/read counters,
zero-retries, and an end-to-end retry-on-connect-error case.
* `pypaimon/tests/rest/client_test.py` — new `HttpClientHttpOptionsTest`:
* Defaults applied when `options=None`
* `http.connect-timeout` / `http.read-timeout` reach `client._timeout`
* `http.keep-alive=false` sets `Connection: close`
* `http.max-connect-retries` / `http.max-read-retries` reach the mounted
adapter's `Retry`
Local: `pytest pypaimon/tests/rest/client_test.py
pypaimon/tests/rest/test_exponential_retry_strategy.py` → 10 passed; `flake8
--config=dev/cfg.ini` clean.
## API and format
`HttpClient.__init__` adds an optional `options` kwarg (backward compatible
— existing `HttpClient(uri)` callers still work and use the same default
behaviour as before, *except* the timeout is now actually honoured).
`ExponentialRetry.__init__` now takes `connect_retries` / `read_retries`
instead of a single `max_retries`. The only internal caller (`token_loader.py`)
is updated; no public API for `ExponentialRetry`.
No file format change.
## Documentation
Option keys are self-described via `with_description(...)` in
`CatalogOptions`. No additional doc change required.
## Generative AI disclosure
Drafted with assistance from an AI coding tool; all logic reviewed by the
author and validated by the tests 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]
