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

Add retry_if_exception_match #125

Closed
PhML opened this issue Jul 9, 2018 · 5 comments
Closed

Add retry_if_exception_match #125

PhML opened this issue Jul 9, 2018 · 5 comments

Comments

@PhML
Copy link

PhML commented Jul 9, 2018

As there is retry_if_exception_type, would it be possible to have retry_if_exception_match?
It would replace :

retry_if_exception(lambda e: re.match(pattern, str(e)))

by:

retry_if_exception_match(pattern)
@immerrr
Copy link
Contributor

immerrr commented Jul 12, 2018

It could be nicer to extend the existing retry_if_exception to accept match kwarg, like pytest.raises does:

retry_if_exception(MyException, match='zzz')
retry_if_exception(MyException, match=some_precompiled_regexp)

@PhML
Copy link
Author

PhML commented Jul 12, 2018

@immerrr I thought about it but since there were already retry_if_exception_type I didn't want to suggest a different convention. Otherwise, it would be more coherent to have:

retry_if_exception(type=MyException)
retry_if_exception(match="pattern")

and so on.
But I think that the purpose of retry_if_exception is to be a base class not a specialized one.

@Brian-Williams
Copy link
Contributor

Brian-Williams commented Jul 12, 2018

I think having a separate classes is a good idea because it gives greater granularity for use cases where several retry predicates need to be chained.

It allows you to do things like separating exception type from exception message.

retry=(retry_if_exception_type=MyException1 | retry_if_exception_message(match=r'a[bc]*'))

vs.

retry=(retry_if_exception_type=MyException1 & retry_if_exception_message(match=r'a[bc]*'))

Then even if exception MyException2 raises with errormessage 'ab' it will retry.

@jd Any interest in this? I put together a rough sketch of what it could look like here.

@jd
Copy link
Owner

jd commented Jul 16, 2018

@Brian-Williams That sounds like a good idea. Feel free to send a PR.

@jamesbraza
Copy link

Should this issue be closed? I think it was resolved

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