Skip to content

Commit 7fbcf68

Browse files
committed
fix: _resolve_pool_knobs preserves explicit 0/0.0 via is-None checks
Caller-provided falsy values (max_conns_per_host=0, idle_timeout=0.0) were silently replaced with spec defaults because of the 'getattr(...) or DEFAULT_*' pattern. Switch to 'is None' so the resolver only substitutes a default when the attribute is missing or explicitly None. Adds TestResolvePoolKnobsExplicitZero with three cases: explicit 0 preserved, missing attrs fall back, explicit None falls back.
1 parent b87c9fb commit 7fbcf68

2 files changed

Lines changed: 62 additions & 3 deletions

File tree

getstream/base.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,17 @@ def _resolve_pool_knobs(obj):
4545
fall back to spec defaults. Top-level ``Stream``/``AsyncStream`` sets
4646
them on ``self`` before calling ``super().__init__()``, so a directly
4747
instantiated sub-client (or test fixture) still gets sane values.
48+
49+
`is None` (not truthiness) so an explicit `0` / `0.0` from the caller is
50+
preserved rather than silently swapped for a default.
4851
"""
52+
max_conns_per_host = getattr(obj, "max_conns_per_host", None)
53+
idle_timeout = getattr(obj, "idle_timeout", None)
54+
connect_timeout = getattr(obj, "connect_timeout", None)
4955
return (
50-
getattr(obj, "max_conns_per_host", None) or DEFAULT_MAX_CONNS_PER_HOST,
51-
getattr(obj, "idle_timeout", None) or DEFAULT_IDLE_TIMEOUT,
52-
getattr(obj, "connect_timeout", None) or DEFAULT_CONNECT_TIMEOUT,
56+
DEFAULT_MAX_CONNS_PER_HOST if max_conns_per_host is None else max_conns_per_host,
57+
DEFAULT_IDLE_TIMEOUT if idle_timeout is None else idle_timeout,
58+
DEFAULT_CONNECT_TIMEOUT if connect_timeout is None else connect_timeout,
5359
)
5460

5561

tests/test_http_client.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,59 @@ async def test_per_call_timeout_reaches_httpx(self):
468468
# ── validation ───────────────────────────────────────────────────────
469469

470470

471+
class TestResolvePoolKnobsExplicitZero:
472+
"""`_resolve_pool_knobs` must use `is None`, not truthiness, so callers
473+
who deliberately pass `0` / `0.0` get their value back unchanged.
474+
Regression test for the falsy-fallback bug flagged in CHA-2956 review.
475+
"""
476+
477+
def test_explicit_zero_preserved(self):
478+
from getstream.base import _resolve_pool_knobs
479+
480+
class Obj:
481+
max_conns_per_host = 0
482+
idle_timeout = 0.0
483+
connect_timeout = 0.0
484+
485+
assert _resolve_pool_knobs(Obj()) == (0, 0.0, 0.0)
486+
487+
def test_missing_attrs_fall_back_to_defaults(self):
488+
from getstream.base import (
489+
_resolve_pool_knobs,
490+
DEFAULT_MAX_CONNS_PER_HOST,
491+
DEFAULT_IDLE_TIMEOUT,
492+
DEFAULT_CONNECT_TIMEOUT,
493+
)
494+
495+
class Obj:
496+
pass
497+
498+
assert _resolve_pool_knobs(Obj()) == (
499+
DEFAULT_MAX_CONNS_PER_HOST,
500+
DEFAULT_IDLE_TIMEOUT,
501+
DEFAULT_CONNECT_TIMEOUT,
502+
)
503+
504+
def test_none_attrs_fall_back_to_defaults(self):
505+
from getstream.base import (
506+
_resolve_pool_knobs,
507+
DEFAULT_MAX_CONNS_PER_HOST,
508+
DEFAULT_IDLE_TIMEOUT,
509+
DEFAULT_CONNECT_TIMEOUT,
510+
)
511+
512+
class Obj:
513+
max_conns_per_host = None
514+
idle_timeout = None
515+
connect_timeout = None
516+
517+
assert _resolve_pool_knobs(Obj()) == (
518+
DEFAULT_MAX_CONNS_PER_HOST,
519+
DEFAULT_IDLE_TIMEOUT,
520+
DEFAULT_CONNECT_TIMEOUT,
521+
)
522+
523+
471524
class TestValidation:
472525
def test_transport_and_http_client_mutually_exclusive(self):
473526
with pytest.raises(ValueError, match="Cannot specify both"):

0 commit comments

Comments
 (0)