Skip to content

[P2][bug] max_retries=0 now raises ConfigurationError at construction - breaking change shipped without a version bump (#115) #121

Description

@karlwaldman

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

What changed

oilpriceapi/retry.py:23-42 (validated_max_retries) now rejects max_retries=0, negatives, and non-ints with ConfigurationError, raised from OilPriceAPI.__init__ / AsyncOilPriceAPI.__init__.

Repro

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

OilPriceAPI(api_key=FIXTURE_KEY, max_retries=0)     # ConfigurationError
OilPriceAPI(api_key=FIXTURE_KEY, max_retries=3.0)   # ConfigurationError

Measured:

{'max_retries': 0}    -> ConfigurationError: max_retries counts total attempts and must be at least 1, got 0.
{'max_retries': -1}   -> ConfigurationError
{'max_retries': True} -> ConfigurationError: ... got bool
{'max_retries': 3.0}  -> ConfigurationError: ... got float

All four constructed successfully before this merge.

Why it matters

The validation itself is correct and worth keeping — silently turning an explicit 0 into 3 was the bug. But this is a breaking change to a public constructor argument, and:

  • oilpriceapi/version.py:8 is still __version__ = "1.13.0" — no bump.
  • max_retries=0 is the natural spelling of "do not retry" and is exactly what an env-driven config produces: int(os.getenv("OPA_MAX_RETRIES", "0")).
  • max_retries=3.0 is what a JSON/YAML config round-trip produces for an integer in many stacks.
  • The failure is at client construction, so it takes the whole process down at startup rather than degrading one call.

A caller upgrading within 1.13.x gets an unannounced crash at import-time-adjacent code.

Suggested fix

Pick one:

  1. Deprecate rather than break. Map 0 -> 1 with a DeprecationWarning naming the new meaning ("max_retries counts total attempts; 0 is being treated as 1"), keep raising for negatives and non-numerics, and accept an integral float (3.0) by coercing. Remove the shim in the next major.
  2. Keep the hard error and bump the version to 2.0.0 (or at minimum 1.14.0 with a prominent CHANGELOG "breaking" entry and a migration line), so the break is visible before it is hit.

Either way this needs a CHANGELOG entry — there is currently nothing telling an upgrading caller that a previously-valid argument is now fatal.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: mediumShould be fixed eventually

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions