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

always use Session.send #250

Merged
2 commits merged into from Jan 23, 2011
Merged

always use Session.send #250

2 commits merged into from Jan 23, 2011

Conversation

minrk
Copy link
Member

@minrk minrk commented Jan 22, 2011

This commit removes all explicit calls to socket.send/socket.recv, and replaces them with session.send/recv. This allows for later expansion of the send/recv protocols, and eases eventual migration to StreamSession object developed in newparallel.

Some adjustments were made to the Session to accomodate this:

  • Session.send can take a Message or dict to allow for multiple sending of the same message
  • Session.send returns a dict instead of wrapping it in a Message
  • Session.recv always returns a tuple of length two: (ident,msg), where ident is None if there was no identity prefix, and msg is None if EAGAIN was raised (indicating no message).

Everything else should function the same.

This allows changes to protocols/patterns
to happen in a single location.
@ellisonbg
Copy link
Member

session.py

  • In send, rename msg_type to msg_or_type to emphasize it can be either.
  • Add docstrings to the send, recv and msg methods, that clarify what the
    methods take and return.
  • Remove the comment on L98.
  • Are you using the Message class in parallel? Should be get away from it?
    What do your recommend? I like that recv returns a raw dict. I think
    session should deal primarily with dicts at its core.

displayhook.py

  • Where was the parent being set before near L17? Was this a bug?

Other than that, this looks good. Why don't you fix these things and go
ahead and merge.

@minrk
Copy link
Member Author

minrk commented Jan 22, 2011

session.py

  • docstrings updated, comment removed.
  • No, I approximately never use the Message class in parallel. I use it once or twice in the Engine (not kernel) and client for attr-access, but nowhere internal.

displayhook.py

The parent setting isn't different at all. Note that L18 is unchanged, and L17 doesn't close its parentheses.

This pull request was closed.
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 this pull request may close these issues.

None yet

2 participants