Skip to content
This repository has been archived by the owner on Oct 26, 2019. It is now read-only.

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

Merged
merged 4 commits into from
Jul 29, 2016

Conversation

jhpedemonte
Copy link
Collaborator

@jhpedemonte jhpedemonte commented Jul 21, 2016

Also adds missing delete test cases using auth token.

Fixes #208

  • REST API to clear cache
  • Command line script to call new REST API
  • Add dashboard upload handling to script
  • Add dashboard deletion handling to script
  • Package script with npm package and update name
  • Update API wiki page (changes)

James Martin and Javier Pedemonte added 2 commits July 21, 2016 15:22
Also adds missing delete test cases using auth token.

Fixes jupyter#208

(c) Copyright IBM Corp. 2016
@parente
Copy link
Member

parente commented Jul 23, 2016

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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

Renames script to jupyter-dashboard-admin

Ref jupyter#208

(c) Copyright IBM Corp. 2016

// command to clear the dashboard server cache
var clear_cache_command = {
command: 'clear-cache [options]',
Copy link
Member

@parente parente Jul 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The common options don't appear when passing --help for me so it's not clear how to pass the host or token without cracking the source. Can we fix this?

EDIT: ... without cracking the source or trying one of the commands without help first.

@parente
Copy link
Member

parente commented Jul 27, 2016

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
Copy link
Collaborator Author

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

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

@parente
Copy link
Member

parente commented Jul 27, 2016

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
Copy link
Collaborator Author

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
Copy link
Member

parente commented Jul 27, 2016

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
Copy link
Collaborator Author

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.

Asks before overwriting existing files/dirs
Better help text.

(c) Copyright IBM Corp. 2016
@jhpedemonte
Copy link
Collaborator Author

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
Copy link
Collaborator Author

Checks should pass once PR #275 is merged.

@parente
Copy link
Member

parente commented Jul 29, 2016

I'll give this a shot tomorrow.

@parente
Copy link
Member

parente commented Jul 29, 2016

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
Copy link
Collaborator Author

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

@jhpedemonte jhpedemonte merged commit b2b140c into jupyter:master Jul 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants