dramatiq icon indicating copy to clipboard operation
dramatiq copied to clipboard

Don't allow Result middleware without a result backend.

Open AndreCimander opened this issue 3 years ago • 3 comments

Since I saw some issues related to the result middleware and django_dramatiq lately, I think it might be sensible to be more strict and descriptive when using the result middleware the wrong way.

My first idea was to use a fallback call like get_broker, but this would also require changes to django_dramatiq which needs to set the backend first, but it's currently way too hot to be really productive and motivated, so raising an descriptive error was my next idea :upside_down_face:

Feel free to adjust the error message, on a second read it sounds a bit harsh.

AndreCimander avatar Aug 08 '20 13:08 AndreCimander

I'm reluctant to make this change since it's technically breaking and, if we were to go in this direction, then I'd rather just make the argument required than have it be optional and error out when it's None. I'll have to think about this some more.

Bogdanp avatar Aug 10 '20 08:08 Bogdanp

Yeah, I have also thought about making the backend parameter required, since the middleware is technically broken without a backend, but you'll lose the ability to give a more specific error message, eventually leading to more confusion for django_dramatiq users.

Maybe just changing django_dramatiq's behaviour to automatically inject the backend into any result-middlewares without a backend is the better approach, but this still has the potential to lead to a broken middleware state if one adds the middleware manually without a backend :thinking:

AndreCimander avatar Aug 10 '20 09:08 AndreCimander

I’ve been thinking about a similar/related scenario: in a codebase I worked with there are oodles of actor functions, some of which return values and some don’t. Without a result backend Dramatiq’s worker issues a warning:

https://github.com/Bogdanp/dramatiq/blob/8dd6a752c63cf89d85a1b265ca707c233b810814/dramatiq/worker.py#L478-L486

Which made me wonder: would you be open to a “NullBackend” which simply ignores results altogether? A bit like the StubBackend. Then we’d (temporarily) add the middleware and drop the result values, maybe log the actor function that attempted to return a result. The NullBackend would be a nice addition to the Dramatiq result backends?

jenstroeger avatar Aug 12 '21 23:08 jenstroeger