tenacity icon indicating copy to clipboard operation
tenacity copied to clipboard

8.5.0 includes breaking changes on the statistics object

Open g3n35i5 opened this issue 1 year ago • 7 comments

With v8.5.0, the statistics object moved from

wrapped_function.retry.statistics to wrapped_function.statistics.

This is a breaking change to the API and thus needs to be a major version update imo.

I'd suggest to add a fallback reference to the wrapped function in order to remove the breaking change.

g3n35i5 avatar Jul 05 '24 14:07 g3n35i5

Seeing something similar. Sanitized sample of our code:

    @tenacity.retry(
        retry=tenacity.retry_if_exception_type(MyCustomError),
        stop=tenacity.stop_after_attempt(MAX_ATTEMPTS),
    )
    def _internal():
        attempt_number = _internal.retry.statistics["attempt_number"]
        ...

now gives this during execution

KeyError: 'attempt_number'

vijaysitaram avatar Jul 08 '24 15:07 vijaysitaram

@hasier is this something we could avoid breaking or should I just bump to version 9?

jd avatar Jul 15 '24 06:07 jd

I'd say it's difficult to keep that API, as per https://github.com/jd/tenacity/pull/484 we need a new retry instance each time the function is called to avoid overwriting the retry context on recursive calls, so that retry object cannot be used for statistics anymore. Unless y'all can think of a different approach, maybe this does deserve a major version bump. Sorry about that.

hasier avatar Jul 15 '24 08:07 hasier

@hasier no worries, this can happen :)

I'm totally fine with the API change, but as you said this should be a major version bump.

g3n35i5 avatar Jul 15 '24 09:07 g3n35i5

@hasier same, gotta do what you gotta do. We ended up working around it in our code by using the previous implementation if it exists or falling back to the newer one, e.g. from attempt_number = _internal.retry.statistics["attempt_number"] to attempt_number = _internal.retry.statistics.get("attempt_number") or _internal.statistics.get("attempt_number")

vijaysitaram avatar Jul 15 '24 19:07 vijaysitaram

@vijaysitaram thanks for the one-liner; this helped us quickly patch our application code.

I agree with others that this should be a major version bump; we just got hit with the same error (and it hits a rare enough code path that we didn't catch it in staging).

steve-marmalade avatar Aug 15 '24 19:08 steve-marmalade

The documentation is outdated regarding this.

Tenacity_—_Tenacity_documentation

osjerick avatar Dec 02 '24 20:12 osjerick