retry_error_callback not copied by Retrying.copy

[markniebur]

The retry_error_callback is not copied by Retry.copy so is omitted when using retry_with. I'm happy to submit a PR with a fix for this, but I'm not sure if this was an oversight or a design choice.

[jd]

Sounds like an error to me.

[akapps]

I also have use cases where I would like to redefine configuration at runtime, using retry_with.

The issue with both retry_error_cls and retry_error_callback not being copied is that both are lost, the new Retrying object using default values.
When defined, those attributes may modify the behavior significantly, raising a different kind of exception, or returning a value rather than raising exceptions. By not reproducing this behavioral signature, the copy becomes surprisingly incompatible with the expected API.

Even if we wouldn't like those properties to be re-defined, IMO they should at least be copied as well.

I prepared a PR inspired by Mark's, with tests to demonstrate the expected behavior and fixing both missing attributes. I open the possibility to redefine those attributes in the copy, accepting the same arguments as the init method, would anyone need it.

[akapps]

I think that this issue is fixed (on master) now.

Do you have any plan regarding when to release a new version ? I would be happy to remove the monkeypatches I currently rely on 😃

[jd]

I'll do this now!