Bogdanp / dramatiq

A fast and reliable background task processing library for Python 3.
https://dramatiq.io
GNU Lesser General Public License v3.0
4.37k stars 313 forks source link

Retries should ignore Shutdown exception from ShutdownNotifications #618

Open jamie-chang-globality opened 7 months ago

jamie-chang-globality commented 7 months ago

Issues

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

Checklist

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 commented 7 months ago

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)