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

retry strategy triggered by any Exception except the ones listed #479

Open
chespinoza opened this issue Nov 4, 2021 · 6 comments
Open
Labels
Issue appropriate for: newcomers 🤩 This issue can be safely tackled by people who never worked in this project before Issue contains: Some documentation 📚 This issues involves writing some documentation Issue contains: Some Python 🐍 This issue involves writing some Python code Issue type: Feature ⭐️ Add a new feature that didn't exist before

Comments

@chespinoza
Copy link

Hi, thanks for this project guys!

I was wondering if is there a way to create a retry strategy that instead of retrying for some listed exceptions it'd retry for any Exception except the ones listed, so basically it's the inverse logic currently used.

I was checking the BaseRetryStrategy class but it seems to me that it doesn't manage the logic for selecting which exceptions should be retried.

Thanks.

@chespinoza chespinoza changed the title retry strategy triggered by any Exception except the ones in a list retry strategy triggered by any Exception except the ones listed Nov 4, 2021
@elemoine
Copy link
Contributor

elemoine commented Nov 5, 2021

I think we could introduce a no_retry_exceptions parameter, and do this in the get_schedule_in function:

    def get_schedule_in(self, *, exception: Exception, attempts: int) -> Optional[int]:
        if self.max_attempts and attempts >= self.max_attempts:
            return None
        if self.retry_exceptions and not isinstance(
            exception,
            tuple(self.retry_exceptions)
        ):
            return None
        if self.no_retry_exceptions and isinstance(
            exception,
            tuple(self.no_retry_exceptions)
        ):
            return None

Not that with this code if the exception is both a "retry exception" and a "no retry exception" then there will be no retry. And if the exception is neither a "retry exception" nor a "no retry exception" there will be no retry either. In other words if there's ambiguity "no retry" is favored over "retry".

@ewjoachim
Copy link
Member

(I think we could raise if retry_exceptions and no_retry_exceptions are used at the same time.)

In the face of ambiguity, refuse the temptation to guess.

@chespinoza
Copy link
Author

could this be implemented just subclassing BaseRetryStrategy then?

@elemoine
Copy link
Contributor

elemoine commented Nov 5, 2021

(I think we could raise if retry_exceptions and no_retry_exceptions are used at the same time.)

Yep, agree.

@ewjoachim
Copy link
Member

could this be implemented just subclassing BaseRetryStrategy then?

Yes I think so. Though it make sense to add this to the procrastinate lib, feel free to implement this on your side in the meantime. And/or to contribute to the lib :)

@ewjoachim ewjoachim added Issue appropriate for: newcomers 🤩 This issue can be safely tackled by people who never worked in this project before Issue contains: Some documentation 📚 This issues involves writing some documentation Issue contains: Some Python 🐍 This issue involves writing some Python code Issue type: Feature ⭐️ Add a new feature that didn't exist before labels Nov 5, 2021
@ewjoachim
Copy link
Member

See https://docs.python.org/3/library/typing.html#typing.overload to properly indicate with mypy that arguments are mutually exclusive

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue appropriate for: newcomers 🤩 This issue can be safely tackled by people who never worked in this project before Issue contains: Some documentation 📚 This issues involves writing some documentation Issue contains: Some Python 🐍 This issue involves writing some Python code Issue type: Feature ⭐️ Add a new feature that didn't exist before
Projects
None yet
Development

No branches or pull requests

3 participants