8.4.2 breaks suggested way to overwrite tenacity behaviors on unit tests

[thiagogodoi]

Looks like the change introduced with 8.4.2: https://github.com/jd/tenacity/compare/8.4.1...8.4.2

Breaks the suggested behavior to overwrite tenacity wait time/retry confiig on tests, as suggested here:

https://github.com/jd/tenacity/issues/106

Doesn't looks like this was done by propose, is it expected? If yes, what would be the suggested way to ovewrite the retry config in tests to speed it up?

[vinayan3]

I'm also seeing this too because the removal of this has thrown off everything

wrapped_f.retry = self # type: ignore[attr-defined]

There is no way to access the retry object now.

[jd]

cc @hasier

[DaveFelce]

Also seeing this after upgrade to 8.4.2. Things like this used to work, but no longer - the test suite which used to take 2 mins to run now takes 12:

external_api_call.retry.stop = stop_after_attempt(1)

[anlambert]

I also got hit by that regression, it seems simply mocking time.sleep is enough now to avoid waiting when testing. It did not resolve the issue of having access to the retry object though.

[robfraz]

Am also hit by this regression. Using an earlier version for tenacity for now.

[hasier]

@jd this use case seems like one more iteration on the fix for the initial scenario https://github.com/jd/tenacity/issues/478. I changed the value for .retry as we need to account for recursive calls to the retriable function. If the value is not copied and therefore shared, then the recursive calls would end up overwriting instance values which affect the operations higher up the stack (this is already a problem with .statistics as well, as they are currently overwritten for the whole function...).

The root cause to me feels like the need to read properties for a decorated function while accounting for the fact that it can be called recursively, in which case the wrapping retry object, statistics, etc. lose a bit of meaning. If we were to tackle this more generally, maybe we'd need to separately track the calls and retry objects in a sort-of-list format, so that each call and their properties can be checked separately? In this case we could keep a writeable retry object for the purpose of this issue, but its read values should not be used (e.g. statistics), we should refer to the new tracking list.

If you prefer a more short-term solution, I feel like there are a couple of different things we could do, depending on your preference:

• Restore the value of retry. This means we need to change the tests (and docs/assumptions) for TestDecoratorWrapper and TestStatistics, as retry should only be accessed to modify the retry object, but not to read its properties, e.g. statistics should be read directly from the function (instead of doing this, I feel maybe we'd be better off tackling the above recursive scenario instead as a more general fix).
• Keep the current value. We'd assume retry is just a special value to call the function (not too sure this is very valuable) and mocks need to be applied differently.

[jd]

I feel like restoring the assignment to self and make it clear that the object access is only meant to read/modify the retry object is the way to go.