Sessionwork #579

Merged
merged 9 commits into from Aug 10, 2011

Projects

None yet

2 participants

@ellisonbg
IPython member

This refactors and adds tests to IPython.zmq.session.Session. The tests can be run with nosetests IPython.zmq.tests.test_session.

  • Renamed the unpack_message method to unserialize.
  • Removed msg_id and msg_type from the top-level message dict (put into headers) throughout the code base.
  • Added tests.
  • Updated the docstrings for Session.
@minrk
IPython member

One thing you might do, since now you are doing many nested dict requests that weren't there before, is add a few msg_id = msg['header']['msg_id'], so you aren't asking for an element of an element of a dict more than you need to.

@ellisonbg
IPython member
ellisonbg added some commits Jul 14, 2011
@ellisonbg ellisonbg Fixing code to assume msg_type and msg_id are top-level.
* I have gone through and looked for instances of ['msg_type'] and
  ['msg_id'] and tried to make sure that I added ['header'] so
  pull the values out of the header.
* But there are many cases where I can't tell if the dict is the
  full message or the header already. This is especially true
  of the msg_id in the parallel db parts of the code.
* Tests pass, but this is scary.
36064ac
@ellisonbg ellisonbg Renaming unpack_message to unserialize and updating docstrings. 6ceaba5
@ellisonbg ellisonbg Fixing docstrings and a few more places for msg_id/msg_type. 1d3fec8
@ellisonbg ellisonbg Adding tests for zmq.session. 28a146d
@ellisonbg ellisonbg More tests for Session.send/recv. 88dae27
@ellisonbg ellisonbg Fixing messaging docs to reflect msg_type in header. 12aef93
@ellisonbg ellisonbg Adding temp refs to msg_type to prevent nested dict gets. ff292f9
@ellisonbg ellisonbg Fixing bug in ipkernel.py related to msg_type refactoring. c4eb828
@ellisonbg
IPython member

This branch should not be merged until after the 0.11 release.

@ellisonbg
IPython member

@minrk: with 0.11 out, can you review this to make sure it is ready to go into master. The htmlnotebook branch is based on this and I would to have this merged so I can follow master again.

@minrk
IPython member

It hasn't changed since I last looked it over, and I just read it over again, so I think it should be fine. Merging this should probably trigger creating the 0.11 maintenance branch.

@ellisonbg ellisonbg merged commit 260396c into ipython:master Aug 10, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment