Add basic functionality (WIP) #1

SaladRaider · 2017-10-01T01:21:29Z

Starter code.

SaladRaider · 2017-10-02T00:50:42Z

@yuvipanda basic functionality seems to be done. I haven't added any tests yet. Wanted to wait on your opinion of how it would be best to write those for this extension. IMO, it looks like unit tests might not be too helpful.

My initial thoughts were to run selenium tests with travis, and then have it check the correctness of the zipped contents.

yuvipanda

This is pretty awesome :)

I think the way to test this would be not to use selenium, but:

Set up a directory layout
Start notebook process with the extension installed in the background
Call the eventstream API
Make sure that the output is what we expect, including finally built zipfile.

This I think will be a lot easier than using selenium!

yuvipanda · 2017-10-02T17:22:53Z

nbzip/static/tree.js

+        $(".col-sm-4.no-padding.tree-buttons").attr('class', 'col-sm-8 no-padding tree-buttons');
+
+        $('#notebook_toolbar .pull-right').prepend(
+          $('<div>').addClass('btn-group').attr('id', 'nbgdrive-link').prepend(


should be nbzip-link maybe?

oops, yeah your right.

yuvipanda · 2017-10-02T17:23:09Z

nbzip/static/index.js

+
+
+    var substatus_messages = [
+        "Adding Hidden Agendas",


hahaha this is cool :D

…puts

SaladRaider · 2017-10-08T22:52:10Z

@yuvipanda thank you for the great idea of how to test this! I tried implementing this approach in the past few commits. The test first tries to check the json output of the eventstream and then checks to see if it can unzip the zipped file and sees if all the correct files are there with the right contents, and there aren't any extra files.

yuvipanda · 2017-10-09T00:30:37Z

Thanks @SaladRaider. The tests look great!

I just realized (thanks to a conversation with @mpacer who pointed it out) there's a better way to do this: On clicking the download button, just generate the zip file in memory and stream it out directly! Does not need intermediate UI, and does not need temporary files to be stored on disk (which will create problems when two zip files are attempted to be downloaded simultaneously).

https://stackoverflow.com/a/21361057 has some example code on how to do this, and I think that'll let us eliminate the whole eventstream + intermediate UI business.

Sorry I didn't think of this earlier, and this might end up undoing some of your work. What do you think of this approach?

SaladRaider · 2017-10-09T00:35:18Z

@yuvipanda yeah, that makes a lot of sense. Should we be worried at all about the files in the notebook exceeding the amount of memory allocated to each user pod? If not, I think this approach will def be better.

yuvipanda · 2017-10-09T00:59:24Z

@SaladRaider ah good question. I think adapting https://stackoverflow.com/a/19121257 to work with tornado's self.write (rather than stdout.write) should solve that problem, allowing us to stream the data out as we construct the zip file!

SaladRaider · 2017-10-15T01:12:43Z

@yuvipanda ok, nbzip should now stream the zip file to the user. The tests are changed as well. I'm also thinking of adding a smaller progress bar to replace the button when you press it to show progress. Or maybe, have a rotating swirly progress icon or something.

yuvipanda · 2017-10-16T17:17:58Z

Cool! Does this need the intermetdiate page at all? Can we completely get rid of it?

SaladRaider · 2017-10-16T17:19:43Z

@yuvipanda the intermediate page is completely gone. I was talking about the visual progress stuff on the /tree page.

mpacer · 2017-10-16T19:56:10Z

Could you explain the use case a bit more… This sounds like it would follow directly from jupyter/notebook#2413 which we are still working on documenting before merging.

yuvipanda · 2017-10-16T19:59:01Z

@mpacer jupyterhub/zero-to-jupyterhub-k8s#152 has more context. That PR in notebook looks fairly unrelated to me.

…

On Mon, Oct 16, 2017 at 12:56 PM, M Pacer ***@***.***> wrote: Could you explain the use case a bit more… This sounds like it would follow directly from jupyter/notebook#2413 <jupyter/notebook#2413> which we are still working on documenting before merging. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB23uxxuzqwMFLPot_InOgOK_XfWZU5ks5ss7TcgaJpZM4Ppv1k> .

-- Yuvi Panda T http://yuvi.in/blog

mpacer · 2017-10-16T19:59:25Z

this chunk of code might be of particular use.

zip_filename = os.path.splitext(name)[0] + '.zip'
handler.set_attachment_header(zip_filename)
handler.set_header('Content-Type', 'application/zip')

# create zip file
buff = io.BytesIO()
with zipfile.ZipFile(buff, mode='w', compression=zipfile.ZIP_STORED) as zipf:
    output_filename = os.path.splitext(name)[0] + resources['output_extension']
    zipf.writestr(output_filename, cast_bytes(output, 'utf-8'))
    for filename, data in output_files.items():
        zipf.writestr(filename, data)

    # pass zip file back
    buff.seek(0)
    handler.finish(buff.getvalue())

yuvipanda · 2017-10-16T19:59:17Z

nbzip/handlers.py

+            # We gonna send out event streams!
+            self.set_header('content-type', 'application/octet-stream')
+            self.set_header('cache-control', 'no-cache')
+            self.set_header('content-disposition', 'attachment; filename=\"{}\"'.format(TEMP_ZIP_NAME))


This should probably be the name of the directory or somesuch?

yeah, that makes sense.

yuvipanda · 2017-10-16T19:59:50Z

nbzip/handlers.py

+
+            self.emit({'output': 'Zipping files:\n', 'phase': 'zipping'})
+
+            q = Queue()


If we have no intermediate page, what is this queue being used for? Am a bit confused :)

i left it for logging, but I guess we don't really need it. I'll remove it.

yuvipanda · 2017-10-16T20:00:31Z

nbzip/static/tree.js

+) {
+
+    // inspired by: https://stackoverflow.com/questions/1106377/detect-when-browser-receives-file-download
+    currToken = null;


var, otherwise this becomes a global variable :)

ok, sounds good. thank you for the catch.

yuvipanda · 2017-10-16T20:01:07Z

nbzip/static/tree.js

+      return new Date().getTime();
+    }
+
+    getCookie = function ( name ) {


I think we can just use the web.authenticated decorator in the python code, and not implement all this code manually.

oh, i'm using the cookie to check if the zipping is done to make the zip button reappear. But, adding the decorator for authentication is a good idea.

mpacer · 2017-10-16T20:06:31Z

In addition, that PR includes code so that the file can be downloaded in the background without needing to use a GET request by using a DataURI attached to a POST request that is downloaded via a simulated click on a hidden anchor triggered once the zipping has been completed.

That way you don't need to implement a download progress bar, you can rely on the native browser's download manager.

You need to use basic xhr requests because otherwise jquery will corrupt the resulting zip file by trying to interpret its encoding in a weird way.

        var xsrf_token = utils._get_cookie("_xsrf");
        var xhr = new XMLHttpRequest();
        xhr.open("POST", url, true);
        xhr.responseType = "blob";
        xhr.withCredentials = true;
        xhr.setRequestHeader("X-XSRFToken", xsrf_token);
        xhr.onreadystatechange = function() {
          if (xhr.readyState === 4) {
            var blob = xhr.response;
            var content_disposition = xhr.getResponseHeader(
              "Content-Disposition"
            );
            var filename = content_disposition.match(/filename\*=utf-8''(.+)/)[1];
            saveData(blob, filename);
          }
        };
        var data = JSON.stringify(that.json_content);
        xhr.send(data);
        return true; // close the dialog
        // return false to keep it open.
      };
      var saveData = (function() {
        var a = document.createElement("a");
        document.body.appendChild(a);
        a.style = "display: none";
        return (
          function(data, fileName) {
            var blob = new Blob([data], { type: "octet/stream" }),
              url = window.URL.createObjectURL(blob);
            a.href = url;
            a.download = fileName;
            a.click();
            window.URL.revokeObjectURL(url);
            document.body.removeChild(a);
          }
        )
      })();

SaladRaider · 2017-10-22T23:51:41Z

@yuvipanda I addressed some of the issues you brought up in this last commit, as well as changed the function of the button to zip the directory the user is currently browsing in.

SaladRaider added 4 commits September 30, 2017 16:52

Add basic functionality

a12618a

Remove old notebook.zip file

e8205cd

Refactor emits

b50cf80

Add useful info and demo to README

30b40bc

yuvipanda reviewed Oct 2, 2017

View reviewed changes

SaladRaider added 5 commits October 7, 2017 12:08

Change html element id to better name

23dc5b6

Add tests for checking json output of eventstream API

ad86fd9

Rename travis file to correct file name

10cac10

Sort contents of eventstream json to correctly compare equivalent out…

ba76fc8

…puts

Add test that checks the integrity of the zipped files

3fd84d2

Stream zip file out to user without creating temp file

2f747a2

yuvipanda suggested changes Oct 16, 2017

View reviewed changes

Allow zipping of sub-dirs and add authentication

54a9b78

yuvipanda approved these changes Nov 13, 2017

View reviewed changes

yuvipanda merged commit d3dba4b into master Nov 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic functionality (WIP) #1

Add basic functionality (WIP) #1

SaladRaider commented Oct 1, 2017

SaladRaider commented Oct 2, 2017

yuvipanda left a comment

yuvipanda Oct 2, 2017

SaladRaider Oct 7, 2017

yuvipanda Oct 2, 2017

SaladRaider commented Oct 8, 2017

yuvipanda commented Oct 9, 2017 •

edited

Loading

SaladRaider commented Oct 9, 2017

yuvipanda commented Oct 9, 2017

SaladRaider commented Oct 15, 2017

yuvipanda commented Oct 16, 2017

SaladRaider commented Oct 16, 2017

mpacer commented Oct 16, 2017

yuvipanda commented Oct 16, 2017 via email

mpacer commented Oct 16, 2017

yuvipanda Oct 16, 2017

SaladRaider Oct 22, 2017

yuvipanda Oct 16, 2017

SaladRaider Oct 22, 2017

yuvipanda Oct 16, 2017

SaladRaider Oct 22, 2017

yuvipanda Oct 16, 2017

SaladRaider Oct 22, 2017

mpacer commented Oct 16, 2017

SaladRaider commented Oct 22, 2017


		self.emit({'output': 'Zipping files:\n', 'phase': 'zipping'})

		q = Queue()

Add basic functionality (WIP) #1

Add basic functionality (WIP) #1

Conversation

SaladRaider commented Oct 1, 2017

SaladRaider commented Oct 2, 2017

yuvipanda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SaladRaider commented Oct 8, 2017

yuvipanda commented Oct 9, 2017 • edited Loading

SaladRaider commented Oct 9, 2017

yuvipanda commented Oct 9, 2017

SaladRaider commented Oct 15, 2017

yuvipanda commented Oct 16, 2017

SaladRaider commented Oct 16, 2017

mpacer commented Oct 16, 2017

yuvipanda commented Oct 16, 2017 via email

mpacer commented Oct 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpacer commented Oct 16, 2017

SaladRaider commented Oct 22, 2017

yuvipanda commented Oct 9, 2017 •

edited

Loading