[P2][bug] The 'or' default fix stopped one line short: timeout=0 still silently becomes 30 (#115)

[karlwaldman]

Merged in #115 (fcc4edf). Verified by execution against main on 2026-09-13 — confirmed live.

#115 fixed two of the three or defaults in the constructor and left the third in place, on the line directly above the fix, in both clients.

oilpriceapi/client.py:120-131 and oilpriceapi/async_client.py:100-108:

self.base_url = (base_url or self.DEFAULT_BASE_URL).rstrip("/")     # <-- unchanged
self.timeout  = timeout or self.DEFAULT_TIMEOUT                     # <-- unchanged
self.max_retries = (self.DEFAULT_MAX_RETRIES if max_retries is None
                    else validated_max_retries(max_retries))        # fixed by #115
self.retry_on = (list(self.DEFAULT_RETRY_CODES) if retry_on is None
                 else list(retry_on))                               # fixed by #115

Repro

from oilpriceapi import OilPriceAPI
FIXTURE_KEY = "fixture-not-a-real-credential"

print(OilPriceAPI(api_key=FIXTURE_KEY, timeout=0).timeout)        # -> 30
print(OilPriceAPI(api_key=FIXTURE_KEY, base_url="").base_url)     # -> https://api.oilpriceapi.com

Measured: timeout=0 becomes 30, base_url="" becomes production.

Why it matters

This is the identical defect the #115 commit message calls out:

The constructor's or defaults discarded explicit configuration: max_retries=0 -> 3, retry_on=[] -> [429, 500, 502, 503, 504]

timeout=0 is meaningful to httpx (fail immediately rather than wait) and is what a config-driven deployment produces from float(os.getenv("OPA_TIMEOUT", "0")). A caller asking for no timeout silently gets a 30-second one, and the resulting hang is very hard to attribute back to the constructor.

Also note self.timeout is passed straight to httpx.Client(timeout=...), so a negative value is accepted and pushed down to httpx unvalidated.

Suggested fix

Same shape as the two lines below it, with validation:

self.timeout = self.DEFAULT_TIMEOUT if timeout is None else validated_timeout(timeout)
self.base_url = ((self.DEFAULT_BASE_URL if base_url is None else base_url) or "").rstrip("/")

where validated_timeout rejects a negative or non-numeric value with ConfigurationError, matching validated_max_retries. If timeout=0 should keep meaning "use the default", that needs to be documented rather than left implicit — but the current behaviour is neither documented nor intentional.