Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8.5.0 includes breaking changes on the statistics object #486

Open
g3n35i5 opened this issue Jul 5, 2024 · 6 comments
Open

8.5.0 includes breaking changes on the statistics object #486

g3n35i5 opened this issue Jul 5, 2024 · 6 comments

Comments

@g3n35i5
Copy link

g3n35i5 commented Jul 5, 2024

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.

@vijaysitaram
Copy link

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'

@jd
Copy link
Owner

jd commented Jul 15, 2024

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

@hasier
Copy link
Contributor

hasier commented Jul 15, 2024

I'd say it's difficult to keep that API, as per #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.

@g3n35i5
Copy link
Author

g3n35i5 commented Jul 15, 2024

@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.

@vijaysitaram
Copy link

vijaysitaram commented Jul 15, 2024

@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")

@steve-marmalade
Copy link

@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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants