Use jupyter nbextension/serverextension for installation/activation #589

ellisonbg · 2016-12-22T04:01:45Z

lgpage · 2016-12-22T10:00:58Z

@ellisonbg, @jhamrick I have post-link and pre-unlink scripts for the conda recipe if you want them.

This should allow the extensions to be installed when nbgrader is installed via conda, no extra commands needed unless the the install flag (--user, --system, --sys-prefix) needs to be different.

ellisonbg · 2016-12-22T16:01:45Z

Yes, one this is merged we should add those to make it easier to install using conda. Let me get the tests passing first...

…

On Thu, Dec 22, 2016 at 3:00 AM, Logan Page ***@***.***> wrote: @ellisonbg <https://github.com/ellisonbg>, @jhamrick <https://github.com/jhamrick> I have post-link and pre-unlink scripts for the conda recipe if you want them. This should allow the extensions to be installed when nbgrader is installed via conda, no extra commands needed unless the the install flag ( --user, --system',--sys-prefix`) needs to be different. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#589 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABr0PchofLX1qhoeM0RsaqIvuS_Z3p0ks5rKkpbgaJpZM4LTnsL> .

-- Brian E. Granger Associate Professor of Physics and Data Science Cal Poly State University, San Luis Obispo @ellisonbg on Twitter and GitHub bgranger@calpoly.edu and ellisonbg@gmail.com

lgpage · 2016-12-22T16:32:07Z

#593 will also need to be merged first for test to work.

ellisonbg · 2016-12-22T17:38:43Z

Tests passing, merging

jhamrick · 2016-12-22T17:42:24Z

@ellisonbg I would prefer if you would have waited for a review from me before merging this. I was in the process of giving a review and I think there are some regressions with the way this is implemented.

jhamrick · 2016-12-22T17:36:47Z

nbgrader/apps/extensionapp.py

-            self.toggle_nbextension("assignment_list/main")
-
-        self.log.info("Done. You may need to restart the Jupyter notebook server for changes to take effect.")
+    $ jupyter serverextension enable --sys-prefix --py nbgrader


Does this have to be activated separately from the nbextension? They go hand in hand and one won't work without the other. Can we make it so the assignment list extension can be fully activated with one command?

The direction we are moving is to keep nbextension/jupyterlab extensions well separated from the server extensions. A couple of reasons for this:

A single server could be serving notebooks to multiple frontends (notebook, nteract, jupyterlab)

We are in the process of separating the notebook server into its own project and repo that won't share any code with the frontends

We are also moving to a different approach for installing all of these things that will make it all easier for users. While we do this, we want to avoid making things more complicated with our existing stuff.

Obviously, in nbgrader, we could wrap all that and expose an endpoint that does it all automatically. For that I think the best approach is to create a conda-forge feedstock with post install scripts that do everything. This is what we are doing with the vega package and the install process is then trivial (no post install commands needed). Are you ok with that approach? Here is the vega-feedstock that has the needed scripts:

https://github.com/conda-forge/vega-feedstock

I see, that makes sense. Having it be done with conda automatically helps, though not everybody installs nbgrader with conda.

I think it's fine if it requires 3 commands to install and activate the assignment list extension as long as we're clear about it. The thing I'm more concerned about is how to install/activate the two extensions (assignment list and create assignment) separately.

jhamrick · 2016-12-22T17:37:40Z

nbgrader/apps/extensionapp.py


+To install for all users, replace `--sys-prefix` by `--system`.


What is the default if --sys-prefix is not included?

Good question! Right now the defaults for serverextensions and nbextensions are different, which we consider to be a bug. However fixing that bug is an API change so it is getting into the code base slowly. Because of this I always give one of the following explicitely: --sys-prefix, --system or --user.

Ok, makes sense.

jhamrick · 2016-12-22T17:38:25Z

nbgrader/apps/extensionapp.py


-    flags = {}
-    aliases = {}
+    $ jupyter nbextension install --sys-prefix --py nbgrader


Is it possible to make it so the different extensions can be installed separately? It will be a regression if this is no longer possible, and I know there are people who want to install e.g. just the "assignment list" extension for their students on a server but not the "create assignment" extension (so students can't easily modify their assignments).

If the commands are run using the --py approach, then it has to be done all together. However, you can do things separately like this:

jupyter nbextension install --sys-prefix --py nbgrader jupyter nbextension enable --sys-prefix create_assignment/main

I can update the documentation in the PR for that usage case.

Ok, so just to be clear, all the extensions will always be installed, but we can give people different instructions for how to selectively enable extensions?

It would be great if you can update the documentation to be really explicit about that -- i.e., have one example for how to install and activate all the extensions, and then another for how to activate just one of the extensions.

jhamrick · 2016-12-22T17:40:10Z

nbgrader/docs/source/user_guide/advanced.rst

@@ -11,71 +11,6 @@ Running nbgrader with JupyterHub

 Please see :doc:`/configuration/jupyterhub_config`.

-.. _assignment-list-installation:


I don't want to remove this documentation, as I think it is still a valid question someone might search for. Rather than deleting it, we should update it with the new installation instructions (i.e. with --system?)

This is already covered in the main installation page. Is that sufficient or do you want to also keep it in this location? The benefit of the main location is that it covers global installation of both the server extension and nbextensions, rather than just the assignment-list stuff. More than willing to put a note back about this.

So the enabling of extensions also works systemwide for all users? If so, I think that should be made more explicit in the main install page (currently it just says you can install the extensions systemwide, but it doesn't make a mention of enabling systemwide). In that case, I think maybe we can move the stuff I flagged below into the main install instructions.

If not, then I think we should definitely keep this section.

Ahh, yes the way I wrote it wasn't clear. Yes, both installation and activation can be done for all users. I will clarify that.

jhamrick · 2016-12-22T17:41:15Z

nbgrader/docs/source/user_guide/advanced.rst

-
-.. warning::
-
-  The "Assignment List" extension is not currently compatible with multiple


This part of the documentation is in particular still relevant.

OK I will create a new section for that and keep them.

jhamrick · 2016-12-22T17:41:32Z

nbgrader/docs/source/user_guide/advanced.rst

-At this point, you should be able to see the "Assignments" tab in the main
-notebook file list.
-
-If you know you have released an assignment but still don't see it in the list


From here on is also still relevant.

ellisonbg · 2016-12-22T17:47:10Z

Go ahead and revert and I can address comments...sorry...

…

On Thu, Dec 22, 2016 at 10:44 AM, Jessica B. Hamrick < ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In nbgrader/apps/extensionapp.py <#589 (review)>: > - def start(self): - if len(self.extra_args) == 0 or "create_assignment" in self.extra_args: - self.log.info("Deactivating create_assignment nbextension") - self.section = "notebook" - self.toggle_nbextension("create_assignment/main") - - if sys.platform != 'win32' and (len(self.extra_args) == 0 or "assignment_list" in self.extra_args): - self.log.info("Deactivating assignment_list server extension") - self.disable_server_extension('nbgrader.nbextensions.assignment_list') - - self.log.info("Deactivating assignment_list nbextension") - self.section = "tree" - self.toggle_nbextension("assignment_list/main") - - self.log.info("Done. You may need to restart the Jupyter notebook server for changes to take effect.") + $ jupyter serverextension enable --sys-prefix --py nbgrader Does this have to be activated separately from the nbextension? They go hand in hand and one won't work without the other. Can we make it so the assignment list extension can be fully activated with one command? ------------------------------ In nbgrader/apps/extensionapp.py <#589 (review)>: > +To install for all users, replace `--sys-prefix` by `--system`. What is the default if --sys-prefix is not included? ------------------------------ In nbgrader/apps/extensionapp.py <#589 (review)>: > - flags = {} - aliases = {} + $ jupyter nbextension install --sys-prefix --py nbgrader Is it possible to make it so the different extensions can be installed separately? It will be a regression if this is no longer possible, and I know there are people who want to install e.g. just the "assignment list" extension for their students on a server but not the "create assignment" extension (so students can't easily modify their assignments). ------------------------------ In nbgrader/docs/source/user_guide/advanced.rst <#589 (review)>: > @@ -11,71 +11,6 @@ Running nbgrader with JupyterHub Please see :doc:`/configuration/jupyterhub_config`. -.. _assignment-list-installation: I don't want to remove this documentation, as I think it is still a valid question someone might search for. Rather than deleting it, we should update it with the new installation instructions (i.e. with --system?) ------------------------------ In nbgrader/docs/source/user_guide/advanced.rst <#589 (review)>: > -.. _assignment-list-installation: - -Installing the "Assignment List" plugin for all students on a server --------------------------------------------------------------------- - -.. seealso:: - - :doc:`installation` - General installation instructions. - - :doc:`managing_assignment_files` - Details on fetching and submitting assignments using the "Assignment List" plugin. - -.. warning:: - - The "Assignment List" extension is not currently compatible with multiple This part of the documentation is in particular still relevant. ------------------------------ In nbgrader/docs/source/user_guide/advanced.rst <#589 (review)>: > -Then, you will need to activate the extension for each student: - -.. code:: bash - - # assuming the $USERS variable contains a list of all your students' - # usernames, e.g. something like - # USERS="jhamrick bitdiddle hacker" - for user in $USERS; do - cd /home/$user - HOME=/home/$user sudo -E -u $user nbgrader extension activate assignment_list - done - -At this point, you should be able to see the "Assignments" tab in the main -notebook file list. - -If you know you have released an assignment but still don't see it in the list From here on is also still relevant. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#589 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABr0LwOcPnxTIyB9Sgw3D7O8G2euJJBks5rKrcIgaJpZM4LTnsL> .

-- Brian E. Granger Associate Professor of Physics and Data Science Cal Poly State University, San Luis Obispo @ellisonbg on Twitter and GitHub bgranger@calpoly.edu and ellisonbg@gmail.com

ellisonbg · 2016-12-22T17:48:19Z

reverted...

jhamrick · 2016-12-22T17:52:31Z

Thanks! I definitely appreciate you implementing this, I would just like to make sure we maintain some of the features that I know people appreciate and have asked about.

jhamrick · 2016-12-22T17:53:44Z

@lgpage I will open another issue for adding those scrips to the conda recipe, thanks!

jhamrick · 2016-12-22T18:10:58Z

nbgrader/docs/source/user_guide/installation.rst

-    nbgrader extension install --help-all
-    nbgrader extension activate --help-all
+* To install for all users, replace `--sys-prefix` by `--system`.
+* To install only for the current user replace `--sys-prefix` by `--user`.


(This is where I mean that it says you can install for all users, but it doesn't explicitly mention enabling for all users)

Yep, will clarify and expand this section as I know lots of folks deploying nbgrader want to do this stuff.

Great, thanks!

jhamrick · 2016-12-22T18:21:21Z

Thanks for updating this, Brian -- it sounds like most of my concerns can be addressed just by updating the docs. I have to go for now but I think with those changes I'll be happy and it'll be good to merge 😄

ellisonbg added 8 commits December 22, 2016 09:41

Adding code in __init__ file to support nbextension/serverextension

ae86a1c

Log the loading of the nbgrader serverextension

1cfa2d9

Remove old nbgrader extension subcommands and provide migration help.

7656001

Updating extension tests

029924e

Updated documentation

6df9412

Fixing tests

182d50e

Getting tests working

2af8de6

Updating wordlist

f143547

ellisonbg force-pushed the nbext branch from 64e091a to f143547 Compare December 22, 2016 16:43

ellisonbg merged commit bbc5905 into jupyter:master Dec 22, 2016

jhamrick reviewed Dec 22, 2016

View reviewed changes

ellisonbg mentioned this pull request Dec 22, 2016

Revert "Use jupyter nbextension/serverextension for installation/activation" #594

Merged

jhamrick mentioned this pull request Dec 22, 2016

Install extensions on conda install #595

Closed

jhamrick reviewed Dec 22, 2016

View reviewed changes

ellisonbg mentioned this pull request Dec 22, 2016

Update nbextension/server extension mechanisms #596

Closed

jhamrick added the refactor label Dec 23, 2016

jhamrick added this to the No action milestone Dec 23, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use jupyter nbextension/serverextension for installation/activation #589

Use jupyter nbextension/serverextension for installation/activation #589

ellisonbg commented Dec 22, 2016

lgpage commented Dec 22, 2016 •

edited

Loading

ellisonbg commented Dec 22, 2016 via email

lgpage commented Dec 22, 2016

ellisonbg commented Dec 22, 2016

jhamrick commented Dec 22, 2016

jhamrick Dec 22, 2016

ellisonbg Dec 22, 2016

jhamrick Dec 22, 2016

jhamrick Dec 22, 2016

ellisonbg Dec 22, 2016

jhamrick Dec 22, 2016

jhamrick Dec 22, 2016

ellisonbg Dec 22, 2016

jhamrick Dec 22, 2016

jhamrick Dec 22, 2016

ellisonbg Dec 22, 2016

jhamrick Dec 22, 2016

ellisonbg Dec 22, 2016

jhamrick Dec 22, 2016

jhamrick Dec 22, 2016

ellisonbg Dec 22, 2016

jhamrick Dec 22, 2016

ellisonbg commented Dec 22, 2016 via email

ellisonbg commented Dec 22, 2016

jhamrick commented Dec 22, 2016

jhamrick commented Dec 22, 2016

jhamrick Dec 22, 2016

ellisonbg Dec 22, 2016

jhamrick Dec 22, 2016

jhamrick commented Dec 22, 2016


		To install for all users, replace `--sys-prefix` by `--system`.

		@@ -11,71 +11,6 @@ Running nbgrader with JupyterHub

		Please see :doc:`/configuration/jupyterhub_config`.

		.. _assignment-list-installation:


		.. warning::

		The "Assignment List" extension is not currently compatible with multiple

Use jupyter nbextension/serverextension for installation/activation #589

Use jupyter nbextension/serverextension for installation/activation #589

Conversation

ellisonbg commented Dec 22, 2016

lgpage commented Dec 22, 2016 • edited Loading

ellisonbg commented Dec 22, 2016 via email

lgpage commented Dec 22, 2016

ellisonbg commented Dec 22, 2016

jhamrick commented Dec 22, 2016

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

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

Choose a reason for hiding this comment

ellisonbg commented Dec 22, 2016 via email

ellisonbg commented Dec 22, 2016

jhamrick commented Dec 22, 2016

jhamrick commented Dec 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhamrick commented Dec 22, 2016

lgpage commented Dec 22, 2016 •

edited

Loading