Integrate the formgrader into the notebook

[jhamrick]

This PR migrates the notebookauth.py and formgrader_extension.py from https://github.com/jhamrick/nbgrader-demo to the official nbgrader repository.

My plan for this is to make nbgrader formgrade actually an alias for launching the notebook, except that instead of automatically popping up the notebook tree, it will open the formgrader interface.

This is the first step towards addressing what has been discussed in #67.

cc @ellisonbg

[ellisonbg]

Very cool, excited about this!

I haven't looked too closely at the code here, but have some questions that maybe @minrk can help with:

First, I was under the impression that serverextensions would automatically inherit the full auth of the notebook server, including its group permissions if run as a service. I think that logic is here:

https://github.com/jupyterhub/jupyterhub/blob/master/jupyterhub/singleuser.py#L44

@minrk is there anything that a serverextension needs to do auth wise? Is there a base handler its handlers should inherit from? I ask because there is still a good amount of auth logic in this PR that I was expecting to go away.

Second, I think it does make sense to have the FormGrade app remain and be an alias to jupyter notebook + visit the formgrade URL. However, I was expecting all configuration of the formgrade server extension to be done on a lower level object (maybe we need a new FormgradeServerExtension object?) that is used in the config file and the actual server extension. IOW, I was expecting the Formgrade app to not be present in the config file or serverextension code. Right now it has things like the ip, port and url stuff that is already on the notebook app.

Cheers, Brian

[jhamrick]

Technically, there isn't actually any "auth" going on here -- all the logic is for how to remap the URLs properly, i.e. to go from /assignments --> /formgrader/assignments. There's nothing in this PR that does any actual authentication or verification of users or anything like that. The "auth" classes are a bit of a misnomer in that regard.

However, I was expecting all configuration of the formgrade server extension to be done on a lower level object (maybe we need a new FormgradeServerExtension object?) that is used in the config file and the actual server extension. IOW, I was expecting the Formgrade app to not be present in the config file or serverextension code. Right now it has things like the ip, port and url stuff that is already on the notebook app.

I'm not sure I totally understand. The formgrader needs to inherit that information from the notebook so that it can correctly transform its handlers and URLs to point to the correct place. Could you maybe give me an example of how you are envisioning doing the configuration?

[jhamrick]

This path can be configured on the server extension side. Can it also be configured through the javascript? If not, then it should probably not be configurable on the server extension side at all, because if it is changed there then this will just break.

[jhamrick]

Oh, I think maybe I understand what you're thinking now. Let me back up and try to explain how everything works currently, and then how I envision it working (this is partially for me too just to put all my thoughts down in the same place).

How it currently works

Currently, when you run nbgrader formgrade, it starts a tornado server with handlers that are defined in nbgrader/formgrader/. These handlers are defined to listen at /, i.e. /assignments, /students, etc.

JupyterHub

When using the formgrader with JupyterHub, we run formgrader's own tornado process and proxy to it using JupyterHub. We then need to perform the following things, which are handled in hubauth.py:

1. Redefine the handlers to listen at, e.g., /services/formgrader rather than /
2. Check whether the users that try to access the formgrader are in fact allowed to access the formgrader.

Notebook

When using the formgrader with the notebook, there is already a tornado server running (the one launched by the notebook). So, rather than proxying as with JupyterHub, we want to just take the handlers defined by the formgrader and add them as handlers on the notebook as well. We then need to perform the following things, which are handled in notebookauth.py:

1. Redefine the handlers to listen at, e.g., /formgrader rather than /

Any actual "authentication" is handled by the notebook server itself (I just tested this though, and currently appears to not the case...).

How it will work

When you run nbgrader formgrade, it will launch a notebook server which loads the "formgrader" server extension. This extension will define all the necessary handlers that are automatically listening at the correct locations, and we will do away with the entire auth/ directory -- that is, the formgrader will become solely a server extension. All authentication will be by the notebook server. To run the formgrader with JupyterHub, you would actually just run the notebook. Group-level permissions will be also handled by JupyterHub and the formgrader will be ignorant of anything to that effect.

[jhamrick]

Ok, @ellisonbg , I think this is perhaps more along the lines of what you were thinking. I have gotten rid of all the auth stuff entirely, and the formgrader is purely a notebook extension. You can access it by clicking the "Formgrader" tab, which is another tree nbextension:

This is actually nice because it lays the groundwork too for incorporating more of the nbgrader stuff in a webapp UI later down the line.

[jhamrick]

Given jupyterhub/jupyterhub#924 , I think this should work out-of-the-box with JupyterHub as long as the server extension is installed. Woohoo, that means I can get rid of all the bulky JupyterHub tests! 🎉

[jhamrick]

Tests are passing. Still need to update the docs.

FYI, I am not planning to merge this until after 0.4.0 is released. I think this is too major a change to make right before a new release... I would rather have it sit on master for a while first to get a bit of battle-testing.

[ellisonbg]

I think that makes sense to delay post 0.4.0.

…

On Sat, Jan 7, 2017 at 9:13 PM, Jessica B. Hamrick ***@***.*** > wrote: Tests are passing. Still need to update the docs. FYI, I am not planning to merge this until after 0.4.0 is released. I think this is too major a change to make right before a new release... I would rather have it sit on master for a while first to get a bit of battle-testing. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#622 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABr0IZcy390XEMLwQv0H8TS87GlLOlnks5rQGJdgaJpZM4LdhEg> .

-- Brian E. Granger Associate Professor of Physics and Data Science Cal Poly State University, San Luis Obispo @ellisonbg on Twitter and GitHub [email protected] and [email protected]