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

EagerBroker #195

Closed
xelhark opened this issue Mar 21, 2019 · 5 comments
Closed

EagerBroker #195

xelhark opened this issue Mar 21, 2019 · 5 comments
Milestone

Comments

@xelhark
Copy link
Contributor

@xelhark xelhark commented Mar 21, 2019

Hi!

Have you ever considered adding an EagerBroker?

My problem is that during test case execution, the join method will hang indefinitely if there is any exception in my code (and since I'm testing, most of the time there actually is one 😄 )

Also, the exceptions aren't bubbled up so I don't see them during the execution of the tests.

I wrote this very simple EagerBroker, but I'm wondering if you think this is good enough, or if there's any better way to handle this kind of situation.

Just in case, I'll leave this here as a reference if anyone googles this:

class EagerBroker(StubBroker):
    def __init__(self):
        super(EagerBroker, self).__init__()

    def process_message(self, message):
        actor = self.get_actor(message.actor_name)
        actor(*message.args, **message.kwargs)

    def enqueue(self, message, *, delay=None):
        self.process_message(message)
@Bogdanp
Copy link
Owner

@Bogdanp Bogdanp commented Mar 21, 2019

It might be worth adding this to the documentation at the very least. I understand the simplicity of having an eager broker but personally think it's a bit of an anti-pattern for integration tests because it changes the operating model of the (presumably controller) code that you're testing.

What I tend to do is join with a timeout on the queue and then investigate if that fails when doing integration tests and unit test my tasks directly to catch things like exceptions. This better preserves the operational model of the code being tested.

@xelhark
Copy link
Contributor Author

@xelhark xelhark commented Mar 21, 2019

I perfectly agree, maybe the best way would be to have a special Worker that bubbles exceptions so that they are raised when calling worker.join('my_queue'), but then again, this would be a special worker class and it might behave differently in production.

@ryanhiebert
Copy link
Contributor

@ryanhiebert ryanhiebert commented Mar 21, 2019

I'm anticipating writing a custom worker for testing a project I'm working on, that would have controls to run the tasks that are currently on the queue and stop after the previously known tasks in the queue are complete, even if more tasks have been enqueued in the process. I'm not sure how relevant that thought is to what you're looking for here, but perhaps there's some overlap.

Bogdanp added a commit that referenced this issue Apr 4, 2019
Closes #195.  This makes it possible to receive the original exception
inside the joining thread.
@Bogdanp
Copy link
Owner

@Bogdanp Bogdanp commented Apr 4, 2019

@xelhark what do you think about https://github.com/Bogdanp/dramatiq/pull/203/files ? This seems like a decent compromise between the two options. The only thing to watch out for is the fact that exceptions are only raised once a task has exhausted its retries.

Bogdanp added a commit that referenced this issue Apr 4, 2019
Closes #195.  This makes it possible to receive the original exception
inside the joining thread.
@Bogdanp Bogdanp added this to the v1.6.0 milestone Apr 4, 2019
@Bogdanp Bogdanp closed this in #203 Apr 5, 2019
@dnmellen
Copy link

@dnmellen dnmellen commented Oct 20, 2020

If anyone likes the "eager" approach for testing I improved a little @xelhark's EagerBroker and I added support for pipelines and groups and GroupCallbacks: https://gist.github.com/dnmellen/17c89389339fdf72e240b2aeb5af4c5a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants