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

Base Message Class? #45

Closed
hugo-zheng opened this issue Sep 25, 2014 · 10 comments
Closed

Base Message Class? #45

hugo-zheng opened this issue Sep 25, 2014 · 10 comments
Assignees
Milestone

Comments

@hugo-zheng
Copy link

hi,
first, thank you for creating such a useful package in python :)
I just start to try out pykka. just a suggestion.
the message pass through actors is an dict, do you think we could have a base message class to extend? I think it might be more powerful than dict.

@jodal
Copy link
Owner

jodal commented Sep 25, 2014

Is there any specific functionality you'd like such a class to provide, or do you simply want a way to mark your own classes as proper messages to be handled by Pykka?

@jodal jodal assigned jodal and unassigned jodal Sep 25, 2014
@drozzy
Copy link

drozzy commented Oct 9, 2014

Actually, I just wanted to suggest a feature similar, but:
This is WRONG. There should be NO "base message" class as that would lead to inheritance. This would lead to mutability in messages.

What we need instead is a "case class" equivalent construct. Something that would create an immutable class instead. See this nice implementation with macropy:
https://github.com/lihaoyi/macropy#case-classes

Unfortunately there is no way to ensure immutability in python (2.x that I know of), so an alternative, like creating a "dictionary" factory might be useful.

For example, here is how I currently handle messages in my store actor:

class StoreMsg(object):
    def __init__(self, taxonomies, envelope, model):
        """
        Message indicating that the given data, corresponding to source
        should be stored in the database.
        :param list[Taxonomy] taxonomies: taxonomies to be used when storing the resource.
        :param SourceContext envelope: an immutable object containing the source
        :param commonframework.json_model.Model model: the model object to be stored
        """
        self.taxonomies = taxonomies
        self.envelope = envelope
        self.command = 'store'
        self.model = model

    @staticmethod
    def match(msg):
        """:param dict msg: other message dict to match to"""
        return msg.get('command', None) == 'store'

    @property
    def msg(self):
        return {'command': self.command,
                'taxonomies': self.taxonomies,
                'envelope': self.envelope,
                'model': self.model
                }

    @staticmethod
    def of(msg):
        """:param dict msg: Store msg dictionary object"""
        return StoreMsg(taxonomies=msg.get('taxonomies'),
                        envelope=msg.get('envelope'),
                        model=msg.get('model'))

Then to create a message I do:
result_store.tell(StoreMsg(taxonomies, self.envelope, model).msg)

and to retrieve a message from the dictionary, I do:

def on_receive(self, msg):
        if StoreMsg.match(msg):
            message = StoreMsg.of(msg)
            ...

NOW, this certainly is a LOT of work compared to, say, Akka equivalent of this:

case class StoreMsg(taxonomies: List[Taxonomy], envelope: Envelope, model: Model)

and then creation is simple:

StoreMessge(taxonomies, envelope, model)

and so is consumption:

def receive = {
   case msg: StoreMsg => ...
}

So I am all in favor of simplifying this process in Pything, just NOT with inheritance. Please.

@drozzy
Copy link

drozzy commented Oct 9, 2014

On the other hand... dictionaries are not immutable either. So I've no idea how to solve this in python.

@drozzy
Copy link

drozzy commented Oct 11, 2014

Would be really nice to support Macropy Case classes instead of plain-ol' dictionaries:
https://github.com/lihaoyi/macropy/blob/master/macropy/case_classes.py

@drozzy
Copy link

drozzy commented Oct 11, 2014

Really, the only thing that needs to change is the "pop" being done on message, which makes the message limited to being a dictionary:
https://github.com/jodal/pykka/blob/master/pykka/actor.py#L199

The whole mechanism of "pykka_reply_to" needs to be scrapped. There should ALWAYS be a "sender" field that should be made available to every actor.(whereas the former mechanism makes it a special thing only available to an actor system)

@drozzy
Copy link

drozzy commented Oct 11, 2014

Found 2nd place where "pop" is used:
https://github.com/jodal/pykka/blob/master/pykka/actor.py#L225

@drozzy
Copy link

drozzy commented Oct 11, 2014

What is your opinion of something like this to wrap around every message?

class Envelope(object):
    """
    Envelope for the message. Contains information relevant to the actor system,
    like the sender and whether the message is an ask message.
    """
    def __init__(self, sender, message, is_ask=False, etc...):
        self.sender = sender
        self.message = message
        self.is_ask = is_ask
        #etc... other actor system message specific properties

This leaves the message to be of any type (serializable?).

@jodal
Copy link
Owner

jodal commented Oct 11, 2014

Python is probably not the right environment if you're looking for real immutability to make you safe. Python's mostly based on responsible consenting adults trying to not do too much wrong.

Some kind of envelope may be a solution. I agree that it would be useful to always expose the sender actor to the receiving actor. That said, I'm more interested in the APIs for telling/asking/receiving than the envelope format itself, as I'd like to be able to provide that data, but keep the APIs dead simple for those not interested in the extra data.

As for serializable messages, yes, that's nice, and of course a requirement for remote actors running on other network hosts (something Pykka doesn't support). Currently though, Pykka doesn't technically require messages to be serializable. We used to do copy.deepcopy() on all messages, but dropped that and put that responsibility on the user of Pykka instead. The reason for this was to not add the performance penalty for those being careful to not change their messages after they were sent. Mopidy, for example, mostly use Pykka with its own brand of immutable objects, similar to macropy's CaseClass.

@drozzy
Copy link

drozzy commented Mar 10, 2015

Just an update on this - as I came up with a way to pass ANY object thought akka. Here is how you can patch the ThreadingActor. Of course, you would inherit from this class instead for creating all your actors:

from pykka import ThreadingActor, ActorRef


class Actor(ThreadingActor):
    """
    Actor class from which classes should inherit to create an actor.

    Adapter to the pykka actors that treats messages as instances
    of some class. If pykka's system message is encountered,
    this adapter simply passes the message on.
    """
    PAYLOAD_MARKER = '__actor.message.payload__'

    def __init__(self, *args, **kwargs):
        super(Actor, self).__init__(*args, **kwargs)
        # Override actor ref with our custom actor ref,
        # that creates a dict each time it sends a message
        self.actor_ref = ActorRefAdaptor(self)

    def _handle_receive(self, message_or_dict):
        if Actor.PAYLOAD_MARKER in message_or_dict:
            # This is our message with an instance of the object in it - pass it
            # straight to on receive function
            self.on_receive(message_or_dict[Actor.PAYLOAD_MARKER])
        else:
            # hand the message over to pykka
            super(Actor, self)._handle_receive(message_or_dict)


class ActorRefAdaptor(ActorRef):
    def tell(self, message):
        # If this is a dict message, it might be pykka's system message,
        # and we leave it as is.
        if isinstance(message, dict):
            super(ActorRefAdaptor, self).tell(message)
        else:
            # Our message is an instance of some class, but we need
            #to pass pykka a dict object - so we wrap it in one!
            super(ActorRefAdaptor, self).tell({Actor.PAYLOAD_MARKER: message})

Now use it simply as:

class MyActor(Actor):
    def on_receive(self, message):
       if isinstance(message, SomeCommand):
           print("Got command with x=%s, and y=%s!!!" % (message.x, message.y))

class SomeCommand(object):
    def __init__(self, x, y):
        self.x = x
        self.y = y

actor = MyActor.start()
actor.tell(SomeCommand(x=1, y=2))

Enjoy!

@jodal jodal added this to the v2.0 milestone Mar 3, 2019
@jodal jodal self-assigned this Mar 3, 2019
@jodal jodal closed this as completed in 97b74fb Mar 3, 2019
@jodal
Copy link
Owner

jodal commented Mar 3, 2019

As mentioned in #39, with the merge of #79 that will be part of the upcoming Pykka 2.0, the last example here now works without having to adapt the actor and ref.

As for adding a sender attribute, see #40.

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