Open blr246 opened 1 month ago
cc @hasier I think you changed that recently π
@blr246 that fix would not work as we use those functions to account for the fact that there might be other implementations for the retry mechanism. By changing those values asyncio
tests don't pass, as we don't wrap the async functions correctly anymore.
@jd I didn't know this was intended behaviour, I can find no tests for it and no mentions in the docs. Personally I'd say the current implementation is what we want to go for and retry mechanisms should derive from the corresponding retry_base
class, which is obviously more annoying than the previous plain function, but helps us better keep the abstraction for the different implementations. If anything, we could check if other
is a callable and wrap it ourselves? It'd be tricky though, in the case of asyncio
there might be other async values we may need to end up checking and break the abstraction to call the asyncio
wrapper (maybe tornado as well?) π
I don't think this was an intended use case indeed and I can imagine people abused it, thanks Python duck typing π Automatic wrapping would be nice I guess, at least for non-asyncio functions if possible.
A recent change subtly breaks existing behavior where users could use operators
&
,|
to compose together instances of the callableretry_base
with regular Python functions. This is useful to avoid the overhead of defining a callable class. This behavior worked as of 8.3.0.The change that appears to be the root cause is: https://github.com/jd/tenacity/commit/21137e79ea6d59907b3b8d236c275c5190a612fe#diff-0dca3b72d66613121507acde32aeb2e6fd42818b367d2fb4189f4180ad09e2f7
Here is a minimal example of what used to be valid and now causes an exception:
Here is the exception:
I believe the following patch fixes the issue (new asyncio seems to support this and does not need to change):