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

Allow passing a custom actor class in @actor decorator #163

Closed
7 tasks done
gilbsgilbs opened this issue Feb 6, 2019 · 3 comments
Closed
7 tasks done

Allow passing a custom actor class in @actor decorator #163

gilbsgilbs opened this issue Feb 6, 2019 · 3 comments
Assignees
Milestone

Comments

@gilbsgilbs
Copy link
Contributor

gilbsgilbs commented Feb 6, 2019

Hi,

Many thanks for working on this project. It's great for those of us who think bloated software is no longer an option (image).

Issues

Short description

It would really help should the @actor decorator took an actor_class argument which would default to Actor. This argument would define the class that will be used to instantiate the actors.

Long description

We are instantiating our workers in a Flask CLI and rely on the app factories pattern. So when all the imports happen (and therefore when the @actor decorator is applied on actors functions), we still have no idea on what the broker should be. We can only instantiate the broker later on, once the config is loaded and the Flask app created.

Then, the Actor.__init__ function seems to produce some side effects on the default broker (the actor is somewhat coupled with a broker, which I don't find great). So once the @actor decorator is applied, it's already too late unfortunately.

Our workaround to this was to create a custom LazyActor class which is declared as follow:

class LazyActor(Actor):
  def __init__(self, *args, **kwargs):
    kwargs.pop('broker')
    self.__args = args
    self.__kwargs = kwargs

  def init_actor(self, broker):
    super().__init__(self, *self.__args, broker=broker, **self.__kwargs)

And then calling the init_actor function with the broker for each actor in our app factory:

[…]
broker = RabbitmqBroker(…)
for lazy_actor in [my_actor_1, my_actor_2, ]:
  lazy_actor.init_actor(broker=broker)
[…]

But this prevent us from using the @actor decorator since objects instantiated by this decorator are forcefully of Actor type. So we had to duplicate the decorator in our code base, just changing Actor to LazyActor which is a bit sad.

If you agree with this tiny change, I could submit a PR. Or maybe you can think of something else that would fix our issue in a better, more elegant way?

From a more general perspective, I think that dramatiq makes slightly too much assumptions at import time. Maybe the Actor class should do things more lazily, once we're sure the user doesn't want to set things up by himself. This is one of the many things I despised in Celery.

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?

Arch, but not relevant.

What version of Dramatiq are you using?

Latest, but not relevant.

What did you do?

See Long Description.

What did you expect would happen?

Not relevant.

What happened?

Not relevant.

@Bogdanp
Copy link
Owner

Bogdanp commented Feb 18, 2019

I'm closing this since the change is now in master.

@isra17
Copy link
Contributor

isra17 commented Nov 15, 2019

I had a similar issue in our project. However I found the actor_class workaround not really working since the decorator still depends on the broker to check for valid options. I have another non-invasive workaround I use available here if it can be of use to anyone. When running a worker you can do LazyActor.load_all() and when using a client, the broker get loaded only once an Actor attribute is used (most of the time, the first all to send).

@Bogdanp
Copy link
Owner

Bogdanp commented Nov 15, 2019

@isra17 cool! I'd be happy to accept that as a contribution to the "cookbook" part of the documentation if you'd be willing to make a PR.

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

No branches or pull requests

3 participants