danielfm / pybreaker

Python implementation of the Circuit Breaker pattern.
BSD 3-Clause "New" or "Revised" License
508 stars 74 forks source link

(🐞) incorrect typing for exclude argument #90

Closed houbie closed 6 months ago

houbie commented 7 months ago

Exclude is defined as: exclude: Sequence[Type[ExceptionType]] | None = None, but it can also be a callable. Mypy complains about following code (when not ignoring the type error):

pybreaker.CircuitBreaker(
        fail_max=CIRCUIT_BREAKER_MAX_FAIL,
        reset_timeout=CIRCUIT_BREAKER_RESET_TIMEOUT,
        # don't trip circuit on client errors, except 429 (too Many Requests)
        exclude=[  # type: ignore[arg-type]
            lambda e: isinstance(e, HTTPError)
            and e.response.status_code < 500
        ],
    )
danielfm commented 7 months ago

Thanks for the report. Would you be willing to submit a pull request to fix the issue?

houbie commented 7 months ago

I'll give it a try...

houbie commented 7 months ago

pre-commit is giving errors :(

I could modernise the python setup like this, guaranteeing reproducible builds. This would also relieve potential contributors from having to manually install tools like pre-commit, black, etc., and they can't forget to run pre-commit install anymore.

Would consider such a pull request?

danielfm commented 7 months ago

Sure, that would be awesome!

houbie commented 7 months ago

I created a draft PR that sets up tooling and fixes the type hints, but I suggest to take it further:

These changes would require python >=3.8, meaning that 3.7 can't be tested anymore (although the lib can still be used with 3.7) Let me know what you think of it

danielfm commented 7 months ago

I'm okay with dropping support for py3.7.

houbie commented 7 months ago

finished the PR: