Opened 12 years ago

Last modified 9 years ago

#8473 closed enhancement

notebook option to upload a .sws file — at Version 21

Reported by: olazo Owned by: iandrus
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: notebook Keywords:
Cc: Merged in:
Authors: Ivan Andrus Reviewers: Karl-Dieter Crisman
Report Upstream: Fixed upstream, but not in a stable release. Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by kcrisman)

It would be great for those of us who have installed Sage to be able to double click on these files and have sage open them in the notebook.

Suporting a command on the likes of:

sage -notebook /path/to/worksheet.sws

would be a very good step in this direction.

Depends on #693; apply only trac_8473-sagenb.2.patch

Syntax is

sage -notebook upload="/path/to/worksheet.sws"

The changes to extcode (specifically for the Mac app) have been moved to #11026.

Change History (24)

comment:1 Changed 12 years ago by olazo

  • Description modified (diff)

comment:2 Changed 12 years ago by mpatel

Ticket #693 is related.

comment:3 Changed 11 years ago by kcrisman

What would be needed for this? #693 shows that it's possible to know whether the notebook is running and then open the browser or not based on this. And http://foo.sagenb.org/home/me/upload_worksheet shows that there is a script to take a .sws file and send it straight to the browser. So I say this is definitely doable by someone who knows these scripts. Is this right?

comment:4 Changed 11 years ago by kcrisman

This also led to the following - #9875.

comment:5 Changed 11 years ago by iandrus

  • Authors set to iandrus
  • Status changed from new to needs_review

Depends on #693

I have added support for uploading from a file:// url in order to support sage --notebook upload=FILE.sws. The sagenb patch allows this, and the extcode patch adds support to the Mac application. I had to support file urls so that the upload url could be given as a GET rather than a POST request, and hence be passed to any browser.

The Mac app has a new option to automatically start the server when a sws file is opened. I'm not sure anyone would ever want to turn this off, so I can remove it if it's not useful.

comment:6 follow-up: Changed 11 years ago by iandrus

  • Owner changed from was to iandrus

I should note I only allow file urls when running on localhost because otherwise it doesn't make much sense and could be a security hole.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 11 years ago by kcrisman

Replying to iandrus:

I should note I only allow file urls when running on localhost because otherwise it doesn't make much sense and could be a security hole.

But I can imagine people definitely wanting to upload an sws file to a Sage instance running only in their browser, i.e. from xyz.sagenb.org. And we already allow arbitrary code in a Sage notebook instance! So I don't know whether this would be any less secure than the current situation...

comment:8 Changed 11 years ago by kcrisman

  • Authors changed from iandrus to Ivan Andrus

comment:9 in reply to: ↑ 7 Changed 11 years ago by iandrus

Replying to kcrisman:

Replying to iandrus:

I should note I only allow file urls when running on localhost because otherwise it doesn't make much sense and could be a security hole.

But I can imagine people definitely wanting to upload an sws file to a Sage instance running only in their browser, i.e. from xyz.sagenb.org. And we already allow arbitrary code in a Sage notebook instance! So I don't know whether this would be any less secure than the current situation...

I was probably unclear. With a file url, it would grab the file from the server rather than the local machine, so it's probably not what people want or expect at all. e.g. if you try to upload file:///home/iandrus/sage/sws1.sws, this will only work if the machine the server is running on also has a file at /home/iandrus/sage/sws1.sws, and then only if the file is the same.

It might be nice to add some way to upload a file to a remote server programmatically, but I think that entails logging in and then sending off a POST request from a script, so it has a different feel. I tried writing a simple script using curl a while ago, but I couldn't get it to work for some reason. Perhaps someone knows of a better/different way to accomplish it.

You are right about the security hole though, it's probably the least of our worries :-)

Changed 11 years ago by iandrus

comment:10 Changed 11 years ago by iandrus

Added urlencoding so that files with special characters will work okay.

comment:11 Changed 11 years ago by kcrisman

I'd like to test this, but I'm reluctant to do so until someone who knows something about the notebook takes a look at # 693. Bug days folks?

Changed 11 years ago by iandrus

comment:12 Changed 11 years ago by iandrus

I added zip and txt files to the Mac App as well as support (icons etc.) for Cython files.

comment:13 follow-up: Changed 11 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman

I can taste it now that you reviewed #693. This is great stuff, good work.

Okay, needs work because this option isn't documented. Ordinarily I wouldn't be as strict with NB stuff because so little of it is documented anyway, but I think it is key that notebook? tells us what happens.

I also have a couple questions, but they just indicate my lack of knowledge.

  • I don't understand this change. More precisely, we now have the default startpath = '', but that isn't the same as \. Will that matter somewhere for non-uploading situations?
                            browse_to(old_interface, old_port, old_secure, '/') 
                            browse_to(old_interface, old_port, old_secure, startpath)
    
  • You said you added something to fix special character issues, but I don't know where that is, other than the place where you import urllib - but you only do that in the case that someone needs to log in. Is that the only place it's needed? This could just be my ignorance speaking.
  • Looks like file names with spaces have to be escaped with the syntax `./sage -n /path/to/work\ sheet.sws'
  • I also discovered what might be a bug, or maybe it's a feature... trying to upload a file via ./sage -n /path/to/sws.sws asks for a new password for admin. That is really annoying, esp. if this is only supposed to work on a local machine anyway!

I'd be inclined to separate the new notebook functionality from adding clickable sws files to the Sage App, for review purposes but also because this would have to be part of a new SageNB package, and it would be nice to separate those things out. The logistics are already complicated enough for things.

comment:14 Changed 11 years ago by kcrisman

  • Status changed from needs_review to needs_work
  • Work issues set to document option, only require login when it should be

This doesn't even work - it asks for a password:

notebook(r'''/Users/.../MAT338Day1-2011.sws''',require_login=False)

while notebook() continues to work fine. I would view this as a bug in the case of this syntax.

comment:15 in reply to: ↑ 13 ; follow-up: Changed 11 years ago by iandrus

Replying to kcrisman:

I can taste it now that you reviewed #693. This is great stuff, good work.

:-)

Okay, needs work because this option isn't documented. Ordinarily I wouldn't be as strict with NB stuff because so little of it is documented anyway, but I think it is key that notebook? tells us what happens.

Good point. I have added documentation for notebook?. Does it need documentation in sage --advanced as well?

I also have a couple questions, but they just indicate my lack of knowledge.

  • I don't understand this change. More precisely, we now have the default startpath = '', but that isn't the same as \. Will that matter somewhere for non-uploading situations?
                            browse_to(old_interface, old_port, old_secure, '/') 
                            browse_to(old_interface, old_port, old_secure, startpath)
    

I don't think that should matter, but I changed it to '/' since it makes more sense. In an earlier version of the patch I think I appended to startpath and that's why it was empty.

  • You said you added something to fix special character issues, but I don't know where that is, other than the place where you import urllib - but you only do that in the case that someone needs to log in. Is that the only place it's needed? This could just be my ignorance speaking.

You are right, there was a bug there. I need it both places. Thanks.

  • Looks like file names with spaces have to be escaped with the syntax `./sage -n /path/to/work\ sheet.sws'

Yes. They have to be escaped for the shell so that they appear as one argument. That's the same as any argument to sage -n. However, I should point out that the syntax is sage -n upload="/path/to work sheet.sws". I guess that's why we need documentation, eh? :-)

  • I also discovered what might be a bug, or maybe it's a feature... trying to upload a file via ./sage -n /path/to/sws.sws asks for a new password for admin. That is really annoying, esp. if this is only supposed to work on a local machine anyway!

I think what's happening here is it's starting a new notebook server with /path/to/sws.sws as it's directory or something. I think that's why it's asking for a password.

I'd be inclined to separate the new notebook functionality from adding clickable sws files to the Sage App, for review purposes but also because this would have to be part of a new SageNB package, and it would be nice to separate those things out. The logistics are already complicated enough for things.

That's probably a good idea. I'll split the extcode off into another ticket.

comment:16 Changed 11 years ago by iandrus

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:17 in reply to: ↑ 15 ; follow-up: Changed 11 years ago by kcrisman

Okay, needs work because this option isn't documented. Ordinarily I wouldn't be as strict with NB stuff because so little of it is documented anyway, but I think it is key that notebook? tells us what happens.

Good point. I have added documentation for notebook?. Does it need documentation in sage --advanced as well?

No, because that says the options are as in notebook() in the command line, which covers it.

  • You said you added something to fix special character issues, but I don't know where that is, other than the place where you import urllib - but you only do that in the case that someone needs to log in. Is that the only place it's needed? This could just be my ignorance speaking.

You are right, there was a bug there. I need it both places. Thanks.

Good.

  • Looks like file names with spaces have to be escaped with the syntax `./sage -n /path/to/work\ sheet.sws'

Yes. They have to be escaped for the shell so that they appear as one argument. That's the same as any argument to sage -n. However, I should point out that the syntax is sage -n upload="/path/to work sheet.sws". I guess that's why we need documentation, eh? :-)

Hah! But it's nice that this also works.

  • I also discovered what might be a bug, or maybe it's a feature... trying to upload a file via ./sage -n /path/to/sws.sws asks for a new password for admin. That is really annoying, esp. if this is only supposed to work on a local machine anyway!

I think what's happening here is it's starting a new notebook server with /path/to/sws.sws as it's directory or something. I think that's why it's asking for a password.

Hmm. So if I use the upload= syntax I should be okay?

comment:18 in reply to: ↑ 17 ; follow-up: Changed 11 years ago by kcrisman

  • Status changed from needs_review to needs_work
  • I also discovered what might be a bug, or maybe it's a feature... trying to upload a file via ./sage -n /path/to/sws.sws asks for a new password for admin. That is really annoying, esp. if this is only supposed to work on a local machine anyway!

I think what's happening here is it's starting a new notebook server with /path/to/sws.sws as it's directory or something. I think that's why it's asking for a password.

Yeah, you're right - I found a random new sagenb folder there!

One thing that happens is that your thing checks whether ANY other Sage server is running with the same folder of worksheets. But I had a different Sage version already running through my usual !sage_notebook.sagenb, and that caused an error since it didn't have the syntax, I guess. I don't know that that's a bug.

I think that the extra / is causing problems, though. Here are two things. The second one works.

Downloads/sage-4.7.alpha1/sage -n upload="/Users/.../MAT338Day1-2011.sws"
Downloads/sage-4.7.alpha1/sage -n upload="Users/.../MAT338Day1-2011.sws"

I think that one too many /s is messing things up. But the first one IS the full path to the file, so this could be very confusing. After all,

Downloads/sage-4.7.alpha1/sage -n upload="~/Downloads/MAT338Day1-2011.sws"

gives

exceptions.IOError: [Errno 2] No such file or directory: '/~/Downloads/MAT338Day1-2011.sws'

So we can't use that syntax for the local directory either. Clearly the extra / isn't helping.

comment:19 in reply to: ↑ 18 Changed 11 years ago by iandrus

Replying to kcrisman:

I think that the extra / is causing problems, though. Here are two things. The second one works.

Downloads/sage-4.7.alpha1/sage -n upload="/Users/.../MAT338Day1-2011.sws"
Downloads/sage-4.7.alpha1/sage -n upload="Users/.../MAT338Day1-2011.sws"

I think that one too many /s is messing things up. But the first one IS the full path to the file, so this could be very confusing. After all,

Downloads/sage-4.7.alpha1/sage -n upload="~/Downloads/MAT338Day1-2011.sws"

gives

exceptions.IOError: [Errno 2] No such file or directory: '/~/Downloads/MAT338Day1-2011.sws'

So we can't use that syntax for the local directory either. Clearly the extra / isn't helping.

Hmm. The extra / doesn't cause a problem on my machine, but I have noticed differences in URL handling on earlier version of OS X, so I'll go ahead a remove it.

Changed 11 years ago by iandrus

comment:20 Changed 11 years ago by kcrisman

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Summary changed from Make .sws files clickable to notebook option to upload a .sws file
  • Work issues document option, only require login when it should be deleted

comment:21 Changed 11 years ago by kcrisman

  • Description modified (diff)
Note: See TracTickets for help on using tickets.