notebook: move preparsing to the worksheet process and out of the server (was: weird pointless line)

[williamstein]

In worksheet.py (at the end of start_next_comp(self)) in sagenb we have these lines:

        self.__comp_is_running = True
        'exec _support_.preparse(base64.b64decode("%s"))'%base64.b64encode(input)
        self.sage().execute(input, os.path.abspath(self.data_directory()))

That 'exec ' line in the middle is just sitting there making a string that is just discarded?

The issue is that I thought I had fully implemented moving all preparsing to the worksheet process (out of the server). Unfortunately, I didn't -- in fact, I only figured out how to do it, but didn't finish.

Component: notebook

Author: William Stein

Reviewer: Mitesh Patel

Merged: sage-4.3.1.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/7483

[williamstein]

comment:2

I think I meant

input = 'exec _support_.preparse(base64.b64decode("%s"))'%base64.b64encode(input)

and to get rid of preparsing input at all done by the server. In fact, I know for a fact I implemented things that way so that this wouldn't be broken:

implicit_multiplication(True)
///
Traceback (most recent call last):
...
NotImplementedError: Implicit multiplication not implemented for the notebook.

But now it seems to be broken again.

I can only conclude that a serious mismerge or mangling has occurred to the code I originally wrote, since I definitely had the above working in an earlier version of sagenb.

Hence, this ticket.

[williamstein]

apply to the sagenb spkg

[williamstein]

Attachment: sagenb_7483.patch.gz

apply to the core sage library

[williamstein]

comment:5

Attachment: sagelib-7483.patch.gz

[williamstein]

comment:6

So the attached patch moves all the preparsing from the notebook server to the worksheet process. I had thought I had made this change long ago, but I definitely hadn't. It's extremely important from a security/robustness/speed/flexibility point of view. Why? Because it means the possibly time consuming and definitely _dangerous_ preparsing work gets pushed off to the worksheet processes, which will often be running on some remote virtual machines. That's what we want!

This patch also makes it so the following are now fully supported in the notebook. Note that they both used to not work at all:

• implicit_multiplication -- turning on and off implicit multiplication

• preparser -- turning on and off the preparser

[williamstein]

comment:7

I've put a new sagenb spkg with just this patch (and the one from 7483) here:

http://wstein.org/home/wstein/patches/sagenb/sagenb-0.4.3.p1.spkg

[qed777]

comment:8

The Selenium test results are unchanged in FF3.5.5 on Linux.

make ptest on sage.math passes.

[qed777]

Reviewer: Mitesh Patel

[qed777]

Author: William Stein

[qed777]

comment:9

It seems that load and save are now broken in the notebook.

Were attach and detach already broken in the notebook?

[williamstein]

comment:10

It seems that load and save are now broken in the notebook.
Were attach and detach already broken in the notebook?

No, this broke them. I'll fix the problem now.

[williamstein]

comment:11

Clarification: load and save still work on .sage files, but don't work on .py. This is related to #4474. But I'll fix it here now, hopefully.

[williamstein]

comment:12

OK, I went a bit crazy and spent 8 hours completely rewriting and unifying and refactoring all the load/save/attach code. This is at #7514. With that, the above mentioned issued is gone.

[qed777]

comment:13

Positive review, once #7514 passes.

Should we also move "docbrowser" generation to a worksheet process?

[williamstein]

comment:14

Should we also move "docbrowser" generation to a worksheet process?

Definitely! The more that is done by worksheet processes, the better -- for scalability, security, etc. Every spec of work done by the server is a potential speed and security problem. The more that can be offloaded to worksheets, the better.

[mwhansen]

comment:16

I've merged the sagelib patch in 4.3.1.alpha0

[williamstein]

comment:17

I've merged this into sagenb-0.4.8.

[kcrisman]

Merged: sage-4.3.1.alpha0