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 to specify an actor class in actor decorator. #169

Closed
wants to merge 1 commit into from

Conversation

@gilbsgilbs
Copy link
Contributor

@gilbsgilbs gilbsgilbs commented Feb 12, 2019

Sorry, I had to move the decorator function down so that I can get the Actor class declared soon enough, which kind of messes up the diff.

Fixes #163

@gilbsgilbs gilbsgilbs force-pushed the gilbsgilbs:actor-class branch 2 times, most recently from 31c4b93 to 2b3e8c1 Feb 12, 2019
@davidt99
Copy link
Contributor

@davidt99 davidt99 commented Feb 14, 2019

I would like to see the LazyActor you mentioned in #163 in this PR so it becomes "official" because I ran into a similar issue.

@@ -220,3 +152,74 @@ def __repr__(self):

def __str__(self):
return "Actor(%(actor_name)s)" % vars(self)


def actor(fn=None, *, actor_name=None, queue_name="default", priority=0, broker=None, actor_class=Actor, **options):

This comment has been minimized.

@Bogdanp

Bogdanp Feb 14, 2019
Owner

Tiny nitpick: please move actor_class before actor_name. All of those are keyword-only arguments so it's not a breaking change and I like how it reads a little better.

This comment has been minimized.

@gilbsgilbs

gilbsgilbs Feb 14, 2019
Author Contributor

Done.

@Bogdanp
Copy link
Owner

@Bogdanp Bogdanp commented Feb 14, 2019

@davidt99 I'd rather we mention this somewhere in the guide rather than include the code for LazyActor.

@gilbsgilbs I just have that one tiny little nitpick. Otherwise, this looks good to me.

@Bogdanp Bogdanp added this to the v1.5.0 milestone Feb 14, 2019
@gilbsgilbs gilbsgilbs force-pushed the gilbsgilbs:actor-class branch 2 times, most recently from aec55ff to 9c792aa Feb 14, 2019
@gilbsgilbs
Copy link
Contributor Author

@gilbsgilbs gilbsgilbs commented Feb 14, 2019

Thanks for the review @Bogdanp .

@gilbsgilbs gilbsgilbs force-pushed the gilbsgilbs:actor-class branch from 9c792aa to d132768 Feb 14, 2019
@Bogdanp
Copy link
Owner

@Bogdanp Bogdanp commented Feb 18, 2019

@gilbsgilbs thanks again. This is now merged with rebase.

@Bogdanp Bogdanp closed this Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants