[WIP] Support for multiple classes via Jupyterhub groups #893

zonca · 2017-09-17T18:21:59Z

Exploring the implementation of support for multiple classes via Jupyterhub groups proposed in #544.

For now only restricts the dropdown of the Assignments List nbextension based on the membership of a student to Jupyterhub groups.

I prepared a ansible setup with users and groups preconfigured to test this at: https://github.com/zonca/jupyterhub-deploy-teaching/tree/nbgrader_test

Add to the documentation

general description on how Jupyterhub groups are used by nbgrader
groups need to be named nbgrader- for students and formgrade- for instructors
Jupyterhub > 0.8 required

zonca · 2017-09-19T13:03:16Z

calling Jupyterhub API with user token requires Jupyterhub 0.8

zonca · 2017-09-19T16:50:39Z

nbgrader/exchange/exchange.py

+        courses = set()
+        for group in groups:
+            if group.startswith('nbgrader-') or group.startswith('formgrade-'):
+                courses.add('-'.join(group.split('-')[1:]))


here I assume that students are in the nbgrader-course_id Jupyterhub group and the instructors in formgrade-course_id group, is this good enough?

You could do:

courses.add(group.split('-', 1)[1])

to get everything after the first - without needing to re-join.

zonca · 2017-09-19T18:57:40Z

nbgrader/exchange/exchange.py

+        if os.getenv('JUPYTERHUB_API_TOKEN'):
+            api_token = os.environ['JUPYTERHUB_API_TOKEN']
+        else:
+            self.exit("JUPYTERHUB_API_TOKEN env is required to run the exchange features of nbgrader.")


@minrk can you please take a look if I'm using the APIs correctly?

Use of the API looks absolutely right to me.

zonca · 2017-09-24T20:42:16Z

The required functionality of the Exchange is implemented. Tests needs to be updated and for that I need to Mock the Jupyterhub APIs.

However, most importantly, I need to add functionality to the add_student and remove_student methods of the Gradebook object so that when students are added or removed from a course database they are also added or removed from the Jupyterhub group (only if it detects we are running inside Jupyterhub).
So I was thinking to add course_id to the constructor of Gradebook and have it stored inside the gradebook.db database (when created in nbgrader/apps/api.py), then used to talk to the Jupyterhub APIs to manage the groups of the users.

@jhamrick @lgpage do you think this is suitable?

zonca · 2017-10-09T10:00:30Z

does anybody have feedback if this way of implementing multiple classes in the assignment list extension is ok?

If not I'll try to go ahead as it is and try to have a complete working version.

jhamrick · 2017-10-09T16:36:50Z

Thanks for working on this! Apologies for not being able to look at this yet—I’ve been on vacation the last few weeks without a computer and haven’t been able to attend to nbgrader issues in the meantime. But, I’ll be back tomorrow and can look at this further soon—I should be able to provide some feedback later this week!

…

On Mon, Oct 9, 2017 at 3:30 PM Andrea Zonca ***@***.***> wrote: does anybody have feedback if this way of implementing multiple classes in the assignment list extension is ok? If not I'll try to go ahead as it is and try to have a complete working version. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#893 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFF9IeLR1F2ESvJmc0MXwTU0R2uUs9Qks5sqe6-gaJpZM4PaOww> .

zonca · 2017-10-09T16:44:04Z

thanks @jhamrick no hurry, even next week is fine!

jhamrick

Thanks, this looks great! I think this way of implementing the multiple classes should be fine.

I like the idea of adding students to the jupyterhub group when they're added to the course database. It should be fine to add the course id as an argument to the constructor of the gradebook, though I would make it an optional argument---some people use nbgrader for a single class without JupyterHub, and we shouldn't force them to have to specify the course id if they're not using that functionality currently.

jhamrick · 2017-10-18T20:30:30Z

nbgrader/exchange/exchange.py

+            user = os.environ['JUPYTERHUB_USER']
+        else:
+            self.exit("JUPYTERHUB_USER env is required to run the exchange features of nbgrader.")
+        import json


Probably would be better to have this import at the top of the file.

jhamrick · 2017-10-18T20:34:58Z

nbgrader/exchange/exchange.py

+        """Check if student is enrolled in course"""
+        from tornado.httpclient import HTTPClient, HTTPRequest
+
+        if os.getenv('JUPYTERHUB_API_TOKEN'):


So just to clarify, this is an environment variable that's automatically set by JupyterHub, even in new terminal environments? So if I launch a terminal from the notebook server and try to run nbgrader list, will that still work appropriately?

yes, correct, but just on Jupyterhub >= 0.8, this feature of nbgrader would not work on 0.7.2

Ok, good to know, we should probably mention that in the docs too.

jhamrick · 2017-10-18T20:36:15Z

nbgrader/exchange/exchange.py

+        groups = json.loads(resp.body.decode('utf8', 'replace'))['groups']
+        courses = set()
+        for group in groups:
+            if group.startswith('nbgrader-') or group.startswith('formgrade-'):


So we're basically making a strong assumption here that all groups that restrict access to nbgrader courses will be prefixed with nbgrader- or formgrade-. That's fine, I just am noting this so it can be added to the documentation here.

ok, I added a list of items to add to the docs in the top post

jhamrick · 2017-10-18T20:38:29Z

nbgrader/server_extensions/assignment_list/handlers.py

@@ -228,7 +228,10 @@ class CourseListHandler(BaseAssignmentHandler):

    @web.authenticated
    def get(self):
-        self.finish(json.dumps(self.manager.list_courses()))
+        courses_list = self.manager.list_courses()
+        if 'value' in courses_list:


This should check the value of the "success" key. If it's false then the value will be a traceback rather than a list.

ok, added the check as if courses_list['success'] and 'value' in courses_list:

zonca · 2017-11-02T15:13:42Z

I have implemented storing the course_id into the Gradebook database. I had to create a new base class in the dbapp classes to create an Exchange object and extract the course_id from it. Now the first time that the db is created from any of the nbgrader db commands, it includes course_id.

Next I'll implement the actual calls to Jupyterhub to set the users, I have the impression that I might not need to store the course_id into the db but read from the configuration file as I am doing now for initializing correctly the Gradebook. I'll find out soon and refactor if needed.

zonca · 2017-11-13T15:14:48Z

Here an example on how this works,

instructor2 releases an assignment:

instructor2@132-249-238-87:~/nbgrader/course2$ nbgrader release course2assign1
[ReleaseApp | INFO] Source: /home/instructor2/nbgrader/course2/release/./course2assign1
[ReleaseApp | INFO] Destination: /srv/nbgrader/exchange/course2/outbound/course2assign1
[ReleaseApp | INFO] Released as: course2 course2assign1

The student is not enrolled, they cannot see it:

student2-course2@132-249-238-87:~$ nbgrader list
[ListApp | WARNING] No nbgrader_config.py file found (rerun with --debug to see where nbgrader is looking)
Call Jupyterhub API /users/student2-course2 post_data: None
[ListApp | INFO] Released assignments:

Same in the extension:

Then we add the student to the class:

instructor2@132-249-238-87:~/nbgrader/course2$ nbgrader db student add student2-course2
[DbStudentAddApp | INFO] Creating/updating student with ID 'student2-course2': {'last_name': None, 'email': None, 'first_name': None
}
Call Jupyterhub API /groups/nbgrader-course2 post_data: None
Call Jupyterhub API /groups/nbgrader-course2/users post_data: {'users': ['student2-course2']}
Student student2-course2 added to Jupyterhub group nbgrader-course2

Now the student can access the assignments:

student2-course2@132-249-238-87:~$ nbgrader list
[ListApp | WARNING] No nbgrader_config.py file found (rerun with --debug to see where nbgrader is looking)
Call Jupyterhub API /users/student2-course2 post_data: None
[ListApp | INFO] Released assignments:
[ListApp | INFO] course2 course2assign1

and

zonca · 2017-11-13T17:17:50Z

@jhamrick I did more testing, I think the implementation is complete. However implement testing is tough. In order to test this functionality I need somehow to mock the calls to the Jupyterhub API.

First idea was to mock the function query_jupyterhub_api, however in the testing the nbserver is running in a separate process, so I don't see a workaround
Second would be to have some actual simple HTTP server that answers to the API calls "pretending to be Jupyterhub". The issue here is that nbgrader will try to authenticate...at this point the HTTP server becomes very complex...

Maybe @minrk has suggestions?

jhamrick · 2017-12-12T22:30:23Z

You could also launch JupyterHub in a separate process---this is actually what I did in the tests for a long time before I turned the formgrader into a notebook server extension. If you're interested in going down this road there is a lot of code you could reuse on the 0.4.x branch. Here is the class I wrote for launching JupyterHub in a separate process:

https://github.com/jupyter/nbgrader/blob/0.4.x/nbgrader/tests/formgrader/manager.py

And also a fake user authenticator and spawner:

https://github.com/jupyter/nbgrader/blob/0.4.x/nbgrader/tests/formgrader/fakeuser.py

zonca · 2017-12-22T13:16:44Z

thanks @jhamrick, it looks like it is going to be a significant effort that unfortunately I cannot make right now. Do you know if somebody else interested in this feature could take it from here and implement the testing part?

sigurdurb · 2018-01-16T14:33:30Z

@zonca Great work, I saw above you tested it with nbgrader in the console, did you also test it through the Formgrader with Jupyterhub, and if so, did it work as expected ?

zonca · 2018-01-16T14:47:14Z

No, I tested with only with the nbgrader extension Il mar 16 gen 2018, 15:33 Sigurður Baldursson <notifications@github.com> ha scritto:

…

@zonca <https://github.com/zonca> Great work, I saw above you tested it with nbgrader in the console, did you also test it through the Formgrader with Jupyterhub, and if so, did it work as expected ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#893 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXYcvk38cgn3w04zfqexHMWeEzqnvNdks5tLLM7gaJpZM4PaOww> .

sigurdurb · 2018-04-30T20:13:29Z

I had some time to test the multiple courses branch from zonca so I merged it with the current master(0.6.0dev)
Really only found 2 issues, one with the formgrader and one that is more nbgrader

Issue: Forbidden api token

When I had an invalid api token which resulted in a forbidden status the nbgrader command line (nbgrader db student add {student_id}) still said it had added it to the jupyterhub group even though really it resulted in a forbidden status. We could simply make and raise an JupyterhubApiException before this line and except it then somewhere here

Issue: No Collect button and Release button does not change.

Collect button dissappears(tested the normal master vs. zonca branch).
After pushing Release it says it is successfull (but no collect button appears). Also the release button does not change into the unrelease X icon.
The assignment still shows up in the assignment list for the students though so this is more a Formgrader issue I guess.
Then the student can submit back to the exchange but the formgrader does not see the submission.
because he has no collect button ofcourse. ( Through the terminal nbgrader collect {assignmentname} works though, that might give some hint where the issue might be)

Thats really the only thing that I see conflicting now, otherwise the flow is very good. I would be happy to see this merged in the next version of nbgrader.
Any ideas where to start looking for a fix for the Issue with the collect and release button ? The logs are not helping nor the browser console. Maybe a similar issue has come up before where things buttons disappear?

zonca · 2018-04-30T21:38:06Z

@sigurdurb nice! I wonder if you can use the browser debugger to check what is going on inside manage_assignments.js:

nbgrader/nbgrader/server_extensions/formgrader/static/js/manage_assignments.js

Line 169 in 2b45dc1

// collect

sigurdurb · 2018-05-02T16:32:37Z

My test setup:

Ubuntu 16.04
jupyterhub version 0.8.1
notebook version 5.4.1
jupyter version 4.4.0
nbgrader version 0.6.0.dev (merged with zoncas multiple_courses branch.)

@zonca It seems like the issue(No Collect button and Release button does not change) gets self–fixed when I have a valid api_token set when I start the service.
At first I thought it was a little bit weird because when I test it with just regular nbgrader I don't need to explicitly give it an api token for the buttons to appear and work. But that is only because the current master is not querying jupyterhub. Even if I take the JUPYTER_API_TOKEN that I get automatically and put it in a simple script to query the jupyterhub api it also results in a forbidden error. Probably the one given by default is just so that the user can query his own service and not jupyterhub.

I am running this as a managed service(starting everything at once)
So starting the service like this now with the api_token set works:

c.Jupyterhub.load_groups = {
     'formgrade-gagr':['sigurdb', 'gagr'],
     'nbgrader-gagr':[],
}
c.JupyterHub.services = [{'name':'gagr20181',
                          'url':'http://127.0.0.1:9993',
                          'api_token':'4d60feb9df9f480db1eb8b77a4ef81ad',
'command':['jupyterhub-singleuser',
           '--group=formgrade-gagr',
           '--debug',
          ],
        'user':'gagr',
        'cwd':'/home/gagr',
}]

I did not notice in my last commit but the logs say that a status of 403(forbidden) is returned from a call to /hub/api/users/<course_id> when I am in the formgrader,
That request that results in the forbidden status can be traced to
exchange.py

And gets called from:
list.py

Also in manage_assignments.js the reason for the icons/buttons not appearing is because every assignment is stuck in the "draft" status. that is because of the info['status'] = 'released' in list.py above
so this line in manage assignments only has draft status
and this line

Tldr: My suggestion would be to replace the print statement in exhange.py with self.log.error give a hint to set the api_token variable, what do you think ?

zonca · 2018-05-02T17:03:24Z

@sigurdurb sure,
also, isn't this triggered if the token is not available? https://github.com/jupyter/nbgrader/pull/893/files#diff-10a5f12d1812bb391fbe4c6298e232d8R58

sigurdurb · 2018-05-02T17:44:11Z

In theory it should but JUPYTER_API_TOKEN is always there in my case, even if I try to set the api_token : None in jupyterhub_config I get an error that it should be a unicode string. Also if I set it as an empty string it automatically gets randomly generated. Also if I skip the api_token it gets automatically set. And like I said above; "Even if I take the JUPYTER_API_TOKEN that I get automatically and put it in a simple script to query the jupyterhub api it also results in a forbidden error. " I tried some more paths from here: https://jupyterhub.readthedocs.io/en/0.7.2/_static/rest-api/index.html but I always get Forbidden. So I am not sure why this JUPYTERHUB_API_TOKEN is always being set by default even though it doesnt work(maybe @minrk can tell us). I guess it makes sense to have it not working because even normal users get a JUPYTERHUB_API_TOKEN so it makes sense that you don't want them to have access to the Api to delete groups or whatever.

meeseeksmachine · 2019-05-09T15:02:35Z

This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/jupyter-nbgrader-hackathon-topics/1040/1

jhamrick · 2019-05-29T22:17:24Z

Thanks so much for all the work you put into this @zonca ! We got @sigurdurb 's PR merged (#1040) which built on top of this one, and are working in the multiple-classes-dev branch to add additional support for having multiple classes even beyond exposing the correct classes to students. I really appreciate the initial work you put into this and I think is going to be a nice foundation for the changes we are making during the Edinburgh hackathon 🙂

I am going to go ahead and close this PR since the other one is now merged.

zonca mentioned this pull request Sep 17, 2017

Assignment List extension does not support multiple classes #544

Closed

zonca force-pushed the multiple_courses branch from a288c9b to 0680787 Compare September 19, 2017 16:43

zonca added 4 commits September 19, 2017 16:48

list courses only if student is in jupyterhub group

c258fe6

implemented function to get courses of authenticated user

ab2e59a

only fetch or submit if user is in the course jupyterhub group

1f36a08

only list assignments user has access to

3256d58

zonca force-pushed the multiple_courses branch from 0680787 to 3256d58 Compare September 19, 2017 16:48

zonca commented Sep 19, 2017

View reviewed changes

simplified, get Jupyterhub user from environment variable

df49640

zonca commented Sep 19, 2017

View reviewed changes

zonca force-pushed the multiple_courses branch from 10be8f4 to df49640 Compare September 19, 2017 19:11

zonca added 2 commits September 20, 2017 18:33

better splitting of group name

0e585c0

[bugfix] run sys.exit

4880698

jhamrick requested changes Oct 18, 2017

View reviewed changes

jhamrick added enhancement refactor labels Oct 29, 2017

jhamrick added this to the 0.6.0 milestone Oct 29, 2017

zonca added 7 commits November 2, 2017 11:43

moved import to top

f37f8cb

check that the call was successful before using the result

481d12c

create table and methods to store course_id

e1c7aa8

set course id in gradebook in the API

10498af

new base class for db to store course_id

bf5b3fd

no need to start exchange, call NbGrader.start

836054b

update id only if not empty string

777a0cd

zonca added 2 commits November 13, 2017 09:22

[bugfix] renamed method

c481722

Renamed to get_user_courses, gets input student_id

3924210

zonca added 2 commits November 13, 2017 15:55

Merge remote-tracking branch 'upstream/master' into multiple_courses

6fbf607

also instructors can access assignment list

9743a21

jhamrick added the needs work label Oct 6, 2018

jhamrick mentioned this pull request Oct 10, 2018

Multiple instructors, multiple courses, and multiple solutions #1030

Closed

sigurdurb mentioned this pull request Oct 31, 2018

Restrict access for students to different courses #1040

Merged

perllaghu mentioned this pull request May 5, 2019

Edinburgh hackathon #1079

Closed

jhamrick added the Edinburgh-Hackathon label May 12, 2019

jhamrick closed this May 29, 2019

jhamrick modified the milestones: 0.6.0, No action May 29, 2019

jhamrick mentioned this pull request Jun 1, 2019

Better support for multiple classes #1131

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Support for multiple classes via Jupyterhub groups #893

[WIP] Support for multiple classes via Jupyterhub groups #893

zonca commented Sep 17, 2017 •

edited

Loading

zonca commented Sep 19, 2017

zonca Sep 19, 2017

minrk Sep 20, 2017

zonca Sep 19, 2017

minrk Sep 20, 2017

zonca commented Sep 24, 2017 •

edited

Loading

zonca commented Oct 9, 2017

jhamrick commented Oct 9, 2017 via email

zonca commented Oct 9, 2017

jhamrick left a comment

jhamrick Oct 18, 2017

zonca Nov 2, 2017

jhamrick Oct 18, 2017

zonca Oct 25, 2017

jhamrick Oct 29, 2017

jhamrick Oct 18, 2017

zonca Nov 2, 2017

jhamrick Oct 18, 2017

zonca Nov 2, 2017

zonca commented Nov 2, 2017

zonca commented Nov 13, 2017 •

edited

Loading

zonca commented Nov 13, 2017

jhamrick commented Dec 12, 2017 •

edited

Loading

zonca commented Dec 22, 2017

sigurdurb commented Jan 16, 2018

zonca commented Jan 16, 2018 via email

sigurdurb commented Apr 30, 2018

zonca commented Apr 30, 2018 •

edited

Loading

sigurdurb commented May 2, 2018

zonca commented May 2, 2018

sigurdurb commented May 2, 2018 •

edited

Loading

meeseeksmachine commented May 9, 2019

jhamrick commented May 29, 2019

[WIP] Support for multiple classes via Jupyterhub groups #893

[WIP] Support for multiple classes via Jupyterhub groups #893

Conversation

zonca commented Sep 17, 2017 • edited Loading

Add to the documentation

zonca commented Sep 19, 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

zonca commented Sep 24, 2017 • edited Loading

zonca commented Oct 9, 2017

jhamrick commented Oct 9, 2017 via email

zonca commented Oct 9, 2017

jhamrick 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

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

zonca commented Nov 2, 2017

zonca commented Nov 13, 2017 • edited Loading

zonca commented Nov 13, 2017

jhamrick commented Dec 12, 2017 • edited Loading

zonca commented Dec 22, 2017

sigurdurb commented Jan 16, 2018

zonca commented Jan 16, 2018 via email

sigurdurb commented Apr 30, 2018

Issue: Forbidden api token

Issue: No Collect button and Release button does not change.

zonca commented Apr 30, 2018 • edited Loading

sigurdurb commented May 2, 2018

zonca commented May 2, 2018

sigurdurb commented May 2, 2018 • edited Loading

meeseeksmachine commented May 9, 2019

jhamrick commented May 29, 2019

zonca commented Sep 17, 2017 •

edited

Loading

zonca commented Sep 24, 2017 •

edited

Loading

zonca commented Nov 13, 2017 •

edited

Loading

jhamrick commented Dec 12, 2017 •

edited

Loading

zonca commented Apr 30, 2018 •

edited

Loading

sigurdurb commented May 2, 2018 •

edited

Loading