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

Retries should ignore Shutdown exception from ShutdownNotifications #618

Open
7 tasks done
jamie-chang-globality opened this issue Apr 11, 2024 · 1 comment
Open
7 tasks done

Comments

@jamie-chang-globality
Copy link

jamie-chang-globality commented Apr 11, 2024

Issues

GitHub issues are for bugs. If you have questions, please ask them on the mailing list.

Checklist

  • Does your title concisely summarize the problem?
  • Did you include a minimal, reproducible example?
  • What OS are you using?
  • What version of Dramatiq are you using?
  • What did you do?
  • What did you expect would happen?
  • What happened?

What OS are you using?

macOS 14.1

What version of Dramatiq are you using?

1.16.0

What did you do?

Setup a dramatiq project with Retries and ShutdownNotification middlewares
Run a tasks on the worker
Interrupt the worker with ctrl-c or otherwise

What did you expect would happen?

No retries being attempted

What happened?

Retries being attempted but shutdown does happen. For async tasks the problem is more severe as the retry is carried out so it delays the shutdown.

Reproduce

Create two actors

@actor
async def async_sleep() -> None:
    while True:
        await asyncio.sleep(5)


@actor
def sync_sleep() -> None:
    while True:
        time.sleep(5)

Run the actors with a broker that includes the following middleware

[
    ShutdownNotifications(True),
    Retries(max_retries=3),
]

and ctrl-c to trigger shutdown

@jamie-chang-globality
Copy link
Author

I've managed to create a workaround by overriding the retries middleware to skip shutdown exceptions

from dramatiq.middleware import Retries as OriginalRetries, Shutdown


class Retries(OriginalRetries):

    def after_process_message(self, broker, message, *, result=None, exception=None):
        if exception and isinstance(exception, Shutdown):
            message.fail()
            return

        return super().after_process_message(broker, message, result=result, exception=exception)

@jamie-chang-globality jamie-chang-globality changed the title Retries should ignore Shutdown exception for Retries should ignore Shutdown exception from ShutdownNotifications Apr 11, 2024
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

1 participant