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

Html notebook #179

Closed
wants to merge 36 commits into from
Closed

Html notebook #179

wants to merge 36 commits into from

Conversation

jamesgao
Copy link

Hi everyone,
I've coded up a preliminary Javascript-based frontend for the new zmq kernel. Fernando helped me get my git set up for this, so a big kudos to him!

I'm new at python setup scripts, but if I did this correctly, you can run this branch with ipython-http. If this did not work, you can try running ipythonhttp.py inside IPython/frontend/html/. As a note, you cannot run the file from the same directory, due to how it extracts the current directory name. Go to IPython/frontend, then run python html/ipythonhttp.py --pylab inline.

As an overview of the architecture:
The HTTP server is just a default BaseHTTPServer, where the BaseHTTPHandler directly hooks into a module-level CometManager object. This object keeps track of any messages the kernel emits, and multiplexes the messages over HTTP to any all clients. The design of IPython.frontend.html.kernelmanager is nearly a direct copy from the qt version. ipythonhttp.py is also a direct copy from the qt.console.ipythonqt. In other words, it is possible to connect the http server to an existing kernel using the exact same commands as in ipython-qtconsole

On the client side, Jquery was used to build most of the javascript objects. Unfortunately, the javascript is poorly tested, and may be fragile. Most of the XREQ and PUB features are supported. Messages on the PUB socket will be displayed by all web clients. This is accomplished with the "Comet" design pattern, where the client opens and maintains a long-living connection to the server in the form of a GET request. The server waits on the kernel for a message, which it immediately sends along. Anything on the PUB socket is transmitted via GET requests, and anything over XREQ is over POST requests. Multi-line inputs are supported with ctrl-enter, with shift-enter to submit. Inline SVGs are supported for pylab inline, and a notebook-like interface for interaction. Tab completion will open a selection window for candidates.

Some known issues:
Major security vulnerability: direct shell access on the internet. Make sure your firewall is secure!
There are no tests for the javascript code
If multiple web clients are connected, message numbers might be messed up
Sometimes, the entry message may disappear without adding another
Some responding "output" messages may get swallowed by the javascript manager (let me know if you can find a reproducible case)

Please give me some feedback on this code!

jamesgao and others added 30 commits October 21, 2010 18:11
…t code really is as simple as I've made it...
… turn escaped terminal commands (colors) into html. Fancy regexps!
… I need to enable the full object_info_request before this is satisfying
queue up messages on the comet side to make execute ok
… will actually display! Also, tweaked the styling
@markvoorhies
Copy link
Contributor

Oops, forgot to preview my last comment. The patch should look like this:

mvoorhie@virgil:~/Cpackages/iPython/ipython$ git diff
diff --git a/setupbase.py b/setupbase.py
index 9d4215d..2530623 100644
--- a/setupbase.py
+++ b/setupbase.py
@@ -111,7 +111,8 @@ def find_packages():
     add_package(packages, 'frontend')
     add_package(packages, 'frontend.qt')
     add_package(packages, 'frontend.qt.console', tests=True)
-    add_package(packages, 'frontend.terminal', tests=True)    
+    add_package(packages, 'frontend.terminal', tests=True)
+    add_package(packages, 'frontend.html')
     add_package(packages, 'kernel', config=False, tests=True, scripts=True)
     add_package(packages, 'kernel.core', config=False, tests=True)
     add_package(packages, 'lib', tests=True)

@fperez
Copy link
Member

fperez commented Oct 22, 2010

James, Mark's comment is the patch we talked about today right before you left. It would be great if you could apply this little fix soon, to make it easier for others to test your code (which is really, really cool :)

@jamesgao
Copy link
Author

Thanks Mark, I've added your patch. As for linking the html files, I'm not sure what's the best solution to this. Currently, the html, css, and javascript files are in the same directory as the main module. The module sets its own basepath using its main.file, and looks for the path from there. Evidently, this does not work with the install script. Since I'm not familiar with the distutils and setup, can someone suggest the best way to make /usr/local/bin/ipython-http work directly?

@fperez
Copy link
Member

fperez commented Oct 22, 2010

I can't help right now (leaving town shortly) but if you don't get help separately, I'll explain how to do it after the weekend...

@ccordoba12
Copy link
Member

I think the page should be named "IPython session" instead of "Ajax Test". It could use the project's logo as favicon too, or maybe just [I]:

Besides, I think the frontend should be named ipython-webnb instead of ipython-http, per the discussion that took place on the dev list a few days ago.

@fperez
Copy link
Member

fperez commented Oct 28, 2010

Hey James,

this looks great, and I really want to get it merged in sooner rather than later, so you don't have to work in isolation for very long.

A few small notes from random testing:

  • you may want to move the code that opens the web browser to happen a little bit later, once you know the kernel is actively listening. Right now by default it tries to open the browser too early, and the user gets an error page. A few seconds later reloading works fine, but it would be better not to show the error page in the first place.
  • Similarly, you should trap KeyboardInterrupt in the main loop, and use that to shut down cleanly. It's a good way to stop the service from a terminal, and likely to be used by regular users. Just catch the KeyboardInterrupt exception, use that as your exit request, and do any shutdown operations you need.
  • You should add the standard copyright notice
#-----------------------------------------------------------------------------
# Copyright (c) 2010, IPython Development Team.
#
# Distributed under the terms of the Modified BSD License.
#
# The full license is in the file COPYING.txt, distributed with this software.
#-----------------------------------------------------------------------------

in the files as you have a chance. It's best that these are carried regularly.

  • Could you also add at the top of the JS files a comment block indicating the intent of the file? I know JS doesn't have docstrings in the Python sense, but a comment block similar to the top-level docstrings will still help. And in general, try to err on the side of verbosity with comments on JS code: most of us in the team know a lot more python than JS, so having the JS code well commented will help us a lot when we dig into it.
  • In html/kernelmanager:
    • Threading module is imported but not needed (you can catch these things by using pyflakes regularly, let me know if you need a hand setting it up; it's a great habit to get into).
    • Try to organize the file a little better overall: imports at the top organized by section, classes, functions, etc. I've added a sample template you can use for reference, to the docs:

http://ipython.scipy.org/doc/nightly/html/development/coding_guide.html#new-files

  • Could you add a reference to the Comet design pattern? I'm not familiar with it, and a URL in the code would help to make sure we look up the same thing you had in mind.
  • Global objects should be clearly declared in a globals section at the top. If some globals are needed that are instances of classes defined in the file itself, then put them all together at the bottom so they stand out clearly (they get easily lost otherwise).
  • Unit tests? Not everything here is easy to auto-test, but objects like the CometManger are quite easy to have tests written for them. Look in the tests/ directories in IPython for test examples, or let me know if you want a hand getting going with this and we can work together. We really, really need to have as tight of a test harness as possible for these codes right off the start.
  • Also add at least some basic docstrings for the various methods, so that other developers know what each of them is for. For user-facing code we want very well documented/formatted docstrings (http://ipython.scipy.org/doc/nightly/html/development/doc_guide.html#docstring-format) but we're not quite that exacting for developer-only code. Still, a basic docstring and parameter explanation goes a long way towards helping everyone be able to work with the code later on.
  • Similarly, could you drop a little reST file in the docs with a basic description of the architecture? Even the text from your pull request (above) would be a great start. In docs/source, add a directory called frontend/ and put in there a file called webnb.txt (to match the top-level script name).
  • Are the various Http*socketChannel classes still missing some implementation work? I see a few 'pass' in there. Do you think you can finish them off before merging? If not that's OK, just mark clearly with #FIXME roughly what's missing, so we know when looking again.

Note: if you need icons, http://ipython.scipy.org/ipython-icons/ has our current set of icons in png format. We can ask Min for the original svg later.

Summary

This is fantastic work. I'm very, very excited about this, and I hope we can get it merged very soon. Let me know if you want a hand with any of the review items above, and we'll get this in. I'll make a note on the list asking for further review on more web-specific stuff, as I know very little about those architectures.

@jamesgao
Copy link
Author

Hi Fernando,
Thanks for your comments! I did some code reshuffling and made everything more clear. I also tightened up message handling on the javascript side, so strange cases like this:

sys.stdout.write("Hello ")
sys.stdout.flush()
time.sleep(2)
sys.stdout.write("World")
sys.stdout.flush()

actually work correctly now. As a side effect (feature!) of this reorganization, now graphs are pushed to all web clients, regardless of who made the request. I'll try to get the documentation and unit-testing up to par over the next few days, although I need to get up to speed on Sphinx.

@fperez
Copy link
Member

fperez commented Oct 28, 2010

Hey James,

thanks, this looks like good progress. One minor point I failed to notice yesterday: we're trying to use pep-8 names by default, which means calling module files with lowercase only: CometManager.py -> cometmanager.py (I'd actually just call it 'comet.py' for short).

the rest of the changes in the last two commits look good. Ping us here when you've completed things and we'll go over it again.

@ellisonbg
Copy link
Member

This is my first round of comment, mostly focusing on the UI.

  • Use a monospaced font for all code and the In/Out prompts. I noticed that the font while you are entering code is not monospaced, but after you are done editing, it becomes monospaced.
  • Less horizontal space between the end of the prompts and the beginning of code and payloads. There is currently about ~10 chars worth of it.
  • The first time you press ctrl-return, it pauses for a second and put an extra space before the next line of input. From then on it doesn't pause to put the extra line in.
  • Use Return for newline, not for code execution. Use shift-return for code execution.
  • The little dot should probably be red when the kernel is busy. I like the green dot for idle though.
  • How do you delete a cell? Insert a new one between existing ones? Reorder cells?
  • I think we should split the Ui into 2 tabs, one tab for the notebook view, which is like the main pane you currently have and another for "timeline" or history view, which would basically be your history side pane. The current history pane will be too small for larger code blocks. This would allow you to make the timeline view look nicer and include plots, etc. It will also keep the main pane less distracting.
  • Should be some sort of title or outer framing to indicate what this is like "IPython Notebook" and maybe even include the ip/port and a link you can email to others.
  • Probably want the ability to collapse large cells and hide the output.
  • When the kernel is busy, we should indicate (with visual highlights) which cell is running.

More comments on the way about the actual code and architecture. Great work though!

@jamesgao
Copy link
Author

Thanks for the comments. A lot of these features are planned for a full notebook interface, which would allow cell shuffling, grouping, etc. That's probably a big enough merge by itself to be a separate pull request. Let's look through this list:

  1. Good call on the monospace font -- the styles are not very tight yet, I have to review what looks best.
    2-3) This sounds like an animation bug, can you send a screenshot? Which browser / version are you running?
  2. Currently, return on a single line will execute, whereas shift-return is required for multi-line input. It's an easy thing to change -- what's the best default?
  3. The dot becomes red when the kernel is "dead", when the COMET requests have ceased.
  4. Deleting a cell is currently accomplished by erasing the entire input, and return. I'm planning to add a context menu that would accomplish this.
  5. UI tabs: I like this idea! I'll probably implement it for a more full fledged notebook interface
    8-9) These features are mostly planned for the full notebook interface
  6. Another great idea! I'll try to add that tonight maybe?

@fperez
Copy link
Member

fperez commented Oct 29, 2010

  1. You almost have the interface that the Qt console has, which is that return executes complete single-line statements, but does newline for incomplete ones (colon at end of line, open quotes, open parens, etc). However, it's very hard to do that in JS, because you need a real python compiler to reliably detect incomplete python statements (that's what we use, the actual compiler).

So I suggest in the NB to simply have, without any smarts:

  • Return: newline always
  • Shift-return: unconditional code execution.

Later, I'd use Ctrl-Return to split up a cell in two, which can be very handy.

@ellisonbg
Copy link
Member

  1. Great
    2-3) This sounds like an animation bug, can you send a screenshot? Which browser / version are you running?

I get the bug in Chrome 7. I will post a screenshot to the list. Also, get a minor issue at the same place with Firefox.

  1. Currently, return on a single line will execute, whereas shift-return is required for multi-line input. It's an easy thing to change -- what's the best default?

I agree with Fernando about Return = newline, Shift-Return=execute code.

  1. The dot becomes red when the kernel is "dead", when the COMET requests have ceased.

Ahh, maybe something like this:

  • Green dot = idle
  • Red dot = busy
  • Invert the entire div to red put "Dead" in while text = dead
  1. Deleting a cell is currently accomplished by erasing the entire input, and return. I'm planning to add a context menu that would accomplish this.

We should have a keyboard stroke dedicated to this...ctrl-D? command-D on Mac?

  1. UI tabs: I like this idea! I'll probably implement it for a more full fledged notebook interface
    8-9) These features are mostly planned for the full notebook interface

Great

  1. Another great idea! I'll try to add that tonight maybe?

Awesome.

@ellisonbg
Copy link
Member

OK, some additional issue:

  1. When I tab complete, I get:

File "/Users/bgranger/Documents/Computation/IPython/code/ipython/IPython/frontend/html/kernelmanager.py", line 95, in do_POST
resp = manager.complete(data["code"].value, data["pos"].value)
File "/Users/bgranger/Documents/Computation/IPython/code/ipython/IPython/frontend/html/CometManager.py", line 62, in complete
chunk = re.split('\s|(|=|;', code[:int(pos)])[-1]
NameError: global name 're' is not defined

Looks like simply an import error.

  1. When it starts up it has two cells showing In[0] and In[1], both blank.

@fperez
Copy link
Member

fperez commented Oct 30, 2010

James, Brian and I had a long talk about the web architecture and things him and Min are doing on the pyzmq side. The short of it is that in a little while, it will likely be possible to refactor some of your http code to run on top of the Tornado web server (imported from pyzmq). This will give us ssl support, a better async model and better scalability.

This is just to give you a heads-up not to worry too much about major work on the http serving part of your code right now. I think if you have a chance to wrap up the various items pointed in this review, we're pretty much ready to merge once those are fixed.

Once merged, we can then continue to build on it and later in the month revisit the http serving part of things. But your code as it is, is a great starting point to build on.

@fperez
Copy link
Member

fperez commented Oct 30, 2010

James, I noticed one more problem: if you actually install your branch, then it doesn't work right. I did an install to a temporary throw-away directory, and in the browser I get:

XML Parsing Error: no element found
Location: http://localhost:8080/notebook
Line Number 1, Column 1:

while the terminal shows:

----------------------------------------
localhost.localdomain - - [29/Oct/2010 21:16:31] "GET /notebook HTTP/1.1" 200 -
----------------------------------------
Exception happened during processing of request from ('127.0.0.1', 57620)
Traceback (most recent call last):
  File "/usr/lib/python2.6/SocketServer.py", line 560, in process_request_thread
    self.finish_request(request, client_address)
  File "/usr/lib/python2.6/SocketServer.py", line 322, in finish_request
    self.RequestHandlerClass(request, client_address, self)
  File "/usr/lib/python2.6/SocketServer.py", line 617, in __init__
    self.handle()
  File "/usr/lib/python2.6/BaseHTTPServer.py", line 329, in handle
    self.handle_one_request()
  File "/usr/lib/python2.6/BaseHTTPServer.py", line 323, in handle_one_request
    method()
  File "/home/fperez/tmp/junk/lib/python2.6/site-packages/IPython/frontend/html/kernelmanager.py", line 60, in do_GET
    page_text = Template(open(basepath + "notebook.html").read())
IOError: [Errno 2] No such file or directory: '/home/fperez/tmp/junk/lib/python2.6/site-packages/IPython/frontend/html/notebook.html'
----------------------------------------

You may need to play with the data_files and/or package_data (I can't remember precisely which right now, have a look at the distutils docs for both of these flags), because I think the problem is that the js/css code isn't getting put in the installed version.

With new code like this, it's always a good idea to test it both from a source tree and from a temporary installation.

@fperez
Copy link
Member

fperez commented Oct 31, 2010

Hey James,

Satra made a good suggestion in general, but the html notebook is probably the most critical place to put it in. How about we add a warning at the top of the opening page for now that reads:

Since the ZMQ code currently has no security, listening on an external-facing IP is dangerous. 
You are giving any computer that can see you on the network the ability to issue arbitrary shell commands as you on your machine. 
Be very careful with this.

Hopefully we can remove it soon, but it's probably a good idea to have in there for now.

@ivanistheone
Copy link

How does the sage notebook handle security?
http://nb.sagemath.org/

very cool!

@jasongrout
Copy link
Member

What is the status of this pull request? It seems very relevant to our new rewrite of the Sage notebook.

@jamesgao
Copy link
Author

Unfortunately, I haven't had enough time to develop this further. I would like to get back into it soon though, so please keep watching. I'm planning to move over to tornado as per the suggestions above.

@jasongrout
Copy link
Member

Several students and I are working on a "single cell Sage notebook" which shares a lot of similarities with your project. At the recent Sage Days 29, I talked a lot with Fernando and others about using the ipython protocol to implement our project. We are implementing our project using uwsgi and flask, so we wouldn't be using websockets, but instead use a database as a huge cache of the requests between the client and server.

Anyways, I'm following this and may use or extend the javascript code here as part of our implementation of the messaging spec. We will probably work on this pretty heavily starting in May.

@ellisonbg
Copy link
Member

The html notebook work is being continued in the htmlnotebook branch here:

https://github.com/ipython/ipython/tree/htmlnotebook

@ellisonbg ellisonbg closed this Mar 31, 2011
@jasongrout
Copy link
Member

Great; thanks!

minrk pushed a commit to minrk/ipython that referenced this pull request Jul 1, 2013
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

7 participants