Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- `HotdataClient` and `ManagedDatabaseClient` accept `request_timeout` (seconds, or a `(connect, read)` pair). The generated SDK otherwise issues every HTTP request with urllib3's no-timeout default, so a stalled or unreachable server blocks the calling thread indefinitely; the new parameter applies a socket-level deadline to every call through the client while still honoring an explicit per-call `_request_timeout`. Also exported as `apply_default_request_timeout(api_client, timeout)` for callers holding a raw generated client. Default remains no timeout (behavior unchanged unless opted in).

## [0.6.2] - 2026-07-08

Expand Down
43 changes: 43 additions & 0 deletions hotdata_framework/client.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import functools
import time
from collections.abc import Iterator
from dataclasses import asdict, dataclass
Expand Down Expand Up @@ -71,6 +72,45 @@ def to_dict(self) -> dict[str, Any]:
return asdict(self)



Comment on lines 73 to +75

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: (not blocking) Three blank lines here — ruff (E303, which this repo selects) wants at most two between top-level definitions. Drop one.

def apply_default_request_timeout(
api_client: ApiClient, timeout: float | tuple[float, float]
) -> None:
"""Give every request through this client a socket-level deadline.

The generated client forwards ``_request_timeout=None`` — urllib3's
no-timeout — on every call unless the caller passes one explicitly, and
most helper methods expose no such knob. A stalled or black-holed server
therefore blocks the calling thread indefinitely. Wrapping the REST seam
applies ``timeout`` (seconds, or a ``(connect, read)`` pair) as the
default while still honoring an explicit per-call ``_request_timeout``.
"""
rest_client = api_client.rest_client
original = rest_client.request

@functools.wraps(original)
def request_with_default_timeout(
method,
url,
headers=None,
body=None,
post_params=None,
_request_timeout=None,
):
Comment on lines +92 to +99

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: (not blocking) This nested function is fully unannotated (params + return), which trips this repo's strict mypy config (disallow_untyped_defs = true). Relatedly, rest_client.request = request_with_default_timeout on line 111 will trigger mypy's [method-assign] ("cannot assign to a method"). CI only runs pytest so the build isn't broken, but the rest of the codebase is strict-mypy clean, so this is a regression against that bar. Consider annotating the signature and adding # type: ignore[method-assign] on the assignment.

Separately, hard-coding the full method, url, headers, body, post_params, _request_timeout signature couples this wrapper to the exact generated RESTClientObject.request shape — a future SDK regeneration that adds a parameter would silently break (dropped kwarg or TypeError). Forwarding via *args, **kwargs (defaulting _request_timeout when absent/None) would survive regeneration.

if _request_timeout is None:
_request_timeout = timeout
return original(
method,
url,
headers=headers,
body=body,
post_params=post_params,
_request_timeout=_request_timeout,
)

rest_client.request = request_with_default_timeout


class HotdataClient:
"""Thin wrapper around the Hotdata Python SDK with query polling helpers."""

Expand All @@ -81,6 +121,7 @@ def __init__(
*,
host: str | None = None,
session_id: str | None = None,
request_timeout: float | tuple[float, float] | None = None,
) -> None:
self._host = normalize_host(host) if host else default_host()
self._api_key = api_key
Expand All @@ -94,6 +135,8 @@ def __init__(
retries=default_http_retries(),
)
self._api = ApiClient(self._config)
if request_timeout is not None:
apply_default_request_timeout(self._api, request_timeout)

@classmethod
def from_env(cls) -> HotdataClient:
Expand Down
2 changes: 2 additions & 0 deletions hotdata_framework/managed_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,15 @@ def __init__(
api_base_url: str,
max_retries: int,
retry_backoff_seconds: float,
request_timeout: float | tuple[float, float] | None = None,
) -> None:
self._max_retries = max_retries
self._retry_backoff_seconds = retry_backoff_seconds
self._runtime = RuntimeClient(
api_key,
workspace_id,
host=api_base_url.rstrip("/"),
request_timeout=request_timeout,
)

def close(self) -> None:
Expand Down
69 changes: 69 additions & 0 deletions tests/test_request_timeout.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
"""The default request timeout must reach every call through the client.

The generated client sends ``_request_timeout=None`` (urllib3 no-timeout)
unless a caller passes one per call — and the helper methods expose no such
knob — so a stalled server blocks the calling thread indefinitely.
``apply_default_request_timeout`` wraps the REST seam once at construction.
"""

from __future__ import annotations

from hotdata_framework.client import HotdataClient, apply_default_request_timeout
from hotdata_framework.managed_client import ManagedDatabaseClient


class _FakeRest:
def __init__(self) -> None:
self.seen: list[object] = []

def request(self, method, url, headers=None, body=None, post_params=None, _request_timeout=None):
self.seen.append(_request_timeout)
return "resp"


class _FakeApi:
def __init__(self) -> None:
self.rest_client = _FakeRest()


def test_default_applied_when_no_per_call_timeout() -> None:
api = _FakeApi()
rest = api.rest_client
apply_default_request_timeout(api, (10.0, 60.0))

assert api.rest_client.request("GET", "http://x") == "resp"
assert rest.seen == [(10.0, 60.0)]


def test_explicit_per_call_timeout_wins() -> None:
api = _FakeApi()
rest = api.rest_client
apply_default_request_timeout(api, 60.0)

api.rest_client.request("GET", "http://x", _request_timeout=5.0)
assert rest.seen == [5.0]


def test_hotdata_client_wraps_its_rest_seam() -> None:
client = HotdataClient("k", "w", host="https://example.test", request_timeout=7.0)
# The wrapper is installed in place of the generated bound method.
assert client.api.rest_client.request.__name__ == "request"
assert client.api.rest_client.request.__wrapped__ is not None


def test_hotdata_client_without_timeout_is_untouched() -> None:
client = HotdataClient("k", "w", host="https://example.test")
assert not hasattr(client.api.rest_client.request, "__wrapped__")


def test_managed_client_threads_timeout_through() -> None:
client = ManagedDatabaseClient(
api_key="k",
workspace_id="w",
api_base_url="https://example.test",
max_retries=1,
retry_backoff_seconds=0.0,
request_timeout=(5.0, 30.0),
)
assert client._runtime.api.rest_client.request.__wrapped__ is not None
client.close()
Loading