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

Recognize actor with module name instead of only func name #640

Closed
7 tasks done
z0z0r4 opened this issue Jul 12, 2024 · 3 comments · Fixed by #641
Closed
7 tasks done

Recognize actor with module name instead of only func name #640

z0z0r4 opened this issue Jul 12, 2024 · 3 comments · Fixed by #641

Comments

@z0z0r4
Copy link
Contributor

z0z0r4 commented Jul 12, 2024

Issues

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

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?

Creating two func decorated with @actor without setting its actor_name in different module, import the first registered func and send message to run, and you may find that dramatiq run the second but not the first func...

It seems dramatiq recognize func defaultly by its func name...

In many cases, the function names in different modules are set to the same. I think that it is an unreasonable design to use the function name as the only index.

actor_name = actor_name or fn.__name__

Should we change the default actor_name to actor_name = actor_name or f'{fn.__module__}.{fn.__name__}'?

I am not familiar with the source code. Based on my initial understanding, it seems that actor_name is only used as the key of the destination function and can be set at will without duplication.

@spumer
Copy link

spumer commented Jul 12, 2024

I think we need to raise error if try to register actor with same name and require change it.

And to auto resolve name conflict is best to use __qualname__ https://devdocs.io/python~3.12/reference/datamodel#function.__qualname__

__qualname__ include full path to func

@z0z0r4
Copy link
Contributor Author

z0z0r4 commented Jul 12, 2024

maybe I will have a PR recently

@shshe
Copy link

shshe commented Jul 12, 2024

We've encountered this problem as well. We added a Dramatiq middleware to detect duplicate actor names:

class EnforceUniqueActorNames(Middleware):
    def before_declare_actor(self, broker, actor):
        if actor.actor_name in broker.actors:
            logger.error(
                "Duplicate actor name detected: %s. Actor names must be unique. "
                "Set a unique name using the `actor_name` kwarg or by renaming the "
                "function decorated by @dramatiq.actor.",
                actor.actor_name,
            )
            raise DuplicateActorError

Wealso use a replacement for the standard @actor decorator that registers the actors with the module name prefix (aligning it with the same behavior as Celery):

def actor_v2(
    fn: Optional[Callable] = None,
    *,
    actor_name: Optional[str] = None,
    **options,
) -> Union[Actor, Callable]:

    def decorator(fn):
        nonlocal actor_name
        if actor_name is None:
            actor_name = fn.__name__
        full_actor_name = f"{fn.__module__}.{actor_name}"
        return actor(
            fn,
            actor_name=full_actor_name,
            **options,
        )

    if fn is None:
        return decorator
    return decorator(fn)

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

Successfully merging a pull request may close this issue.

3 participants