jupyter / dashboards_server

[RETIRED] Server that runs and renders Jupyter notebooks as interactive dashboards
Other
181 stars 48 forks source link

Adds REST API allow clearing of all or some cached dashboards #265

Closed jhpedemonte closed 8 years ago

jhpedemonte commented 8 years ago

Also adds missing delete test cases using auth token.

Fixes #208

parente commented 8 years ago

Does it make sense to rename the clear cache script to jupyter-dashboards-admin and support other ops like deletion, no just cache clearing?

Does the script get bundled into the npm package? There's a make target for building a release without sending it to npm I think.

The API wiki page should get updated.

jhpedemonte commented 8 years ago

As in deletion of a notebook/dashboard? Sure, we can add that. Could add upload as well.

We did remember the API wiki page, just haven't done that yet. And we'll take a look at npm package and naming.

jhpedemonte commented 8 years ago

There's just the npm publish. But I was able to verify using npm pack that everything in /bin is included.

parente commented 8 years ago

Few problems using the CLI for uploads:

  1. Uploaded notebook does not appear on disk or in the dashboard listing. Not sure where it went.
cd jupyter_dashboards/bin
./jupyter-dashboard-admin upload ../etc/notebooks/test/test_layout.ipynb test/test_layout2.ipynb
Dashboard successfully uploaded!
ls -l ../data/test
# test_layout2.ipynb is not here
  1. File upload completely replaces an existing directory of the same name. This should probably give a warning or error in the CLI vs allowing the overwrite like we do from the bundler. (Why: the human is giving the path here and can mess it up vs the bundler code in the original case.)
cd jupyter_dashboards/bin
./jupyter-dashboard-admin upload ../etc/notebooks/test/test_layout.ipynb test
Dashboard successfully uploaded!
ls -l ../data
# test/ folder is completely deleted, only test notebook remains
  1. Notebook never uploaded to non-existent subpath. Subpath gets created, but notebook / bundle is not written to disk.
cd jupyter_dashboards/bin
./jupyter-dashboard-admin upload ../etc/notebooks/test/test_layout.ipynb new_folder/test_layout.ipynb
Dashboard successfully uploaded!
ls -l ../data/new_folder
# nothing in the folder
jhpedemonte commented 8 years ago

File upload completely replaces an existing directory of the same name.

This is the standard behavior of the REST API for /upload.

parente commented 8 years ago

This is the standard behavior of the REST API for /upload.

Yes. Understood. I was poorly explaining that the behavior is correct for the bundler type action where a user just clicks "Deploy to dashboard server" and the right thing happens. But from a CLI, where a human has to type a file name, we might want the CLI to at least give a warning if something with that name already exists because the human made a typo or some such.

jhpedemonte commented 8 years ago

Without changing the current REST API, we could warn the user that a dashboard by that name already exists (whether an individual file or bundled) and ask if user wants to continue. Does that seem fine to you? Or did you have something else in mind?

parente commented 8 years ago

Without changing the current REST API, we could warn the user that a dashboard by that name already exists (whether an individual file or bundled) and ask if user wants to continue.

Yea. That's what I was thinking. Something simple in the CLI by doing a GET first to check existence. I definitely don't want to change the API.

jhpedemonte commented 8 years ago

Uploaded notebook does not appear on disk or in the dashboard listing

This is because the destination name is listed with the .ipynb extension. It's a bug that it says success when nothing happened. But I also need to make it more evident in the help that the destination should be a dashboard name, no extension.

jhpedemonte commented 8 years ago

OK, I think I took care of all of the issues found by Pete. @parente and @jameslmartin, please take a look at latest changes.

Also, I updated the wiki.

jhpedemonte commented 8 years ago

Checks should pass once PR #275 is merged.

parente commented 8 years ago

I'll give this a shot tomorrow.

parente commented 8 years ago

Tested locally. Everything looks good now.

@jhpedemonte You do the merge since you're also working on #275. I don't know if you want to fix that first or get this in or ...

jhpedemonte commented 8 years ago

Reran Travis tests since PR #278 went in. Now green so merging.