[IPython-dev] Code review of html frontend
Brian Granger
ellisonbg at gmail.com
Fri Oct 29 13:44:22 EDT 2010
James,
Here is a longer code review of the html frontend. Overall, this is
really nice. Great job!
====================
HTML Notebook Review
====================
General
=======
* Add IPython standard copyright to each file and put an
author section with yourself in the module level docstrings.
* Use a space after the "#" in comments.
* We should refactor the main KernelManager to consist of two
classes, one that handles the starting of the kernel process
and a second that handles the channels. In this case, there
is not reason to have all the channel stuff, as I think
we can simple use comet or websockets to pass the raw
ZMQ JSON messages between the client and the kernel. The
only reason we might not want to do this is to allow us
to validate the messages in the web server, but really we
should be doing that in the kernel anyways. This would
make the webserver stuff even thinner.
* Let's document clearly the URL structure we are using. It
will be much easier to do this if we move to tornado.
* Let's make sure we develop the html/js stuff in a way that
will be independent of the webserver side of things, that
way, in the future, we can easily swap out either component
if we want.
* Please add brief docstrings to the important methods.
* We will probably have multiple html frontends, so we should
probably put your stuff in something like html/basic or
html/basicnb. We did this for the qt frontend (qt/console).
* For now I would mostly focus on the Javascript and UI side of
things, as I think we probably want to move to Tornado
even for simple servers like this.
UI
==
* The results of ? and ?? only go to one client.
ipythonhttp.py
==============
* We should discuss the possibility of using Tornado as our
web server. It will be a much better match for working
with zmq and many things will be much easier. We are
already shipping a good part of Tornado with pyzmq and
could possibly ship all of it. Using the stdlib for now
is fine though. Tornado also has websocket support that
would work extremely well with pyzmq.
* Remove command line options that you don't support yet.
* Move defer to the top level and rename something like
"start_browser". Move import threading to top level as well.
kernelmanager.py
================
* In do_GET and do_POST, document what each elif clause is
handling.
* I see that you are using time.time for the client id. Would
it be better to use a real uuid? If you want to also have
the time you could still pass that to manager.register as
well.
* When a kernel dies, the client should note that and there
should be an option to restart the kernel.
* We should probaby have top level URLs for the different
ZMQ sockets like /XREQ, /SUB, etc. so that the GET and POST
traffic has different URLS.
CometManager.py
================
* Change filename to cometmanager.py.
--
Brian E. Granger, Ph.D.
Assistant Professor of Physics
Cal Poly State University, San Luis Obispo
bgranger at calpoly.edu
ellisonbg at gmail.com
More information about the IPython-dev
mailing list