Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

View Feedback link not working whin in lab mode #1664

Closed
perllaghu opened this issue Sep 16, 2022 · 15 comments · Fixed by #1675
Closed

View Feedback link not working whin in lab mode #1664

perllaghu opened this issue Sep 16, 2022 · 15 comments · Fixed by #1675

Comments

@perllaghu
Copy link
Contributor

Dockerized notebooks running in a Kubernetes cluster

I'm testing NBgrader 0.8 for our service, and everything is working fine (including our external exchange service) - however the View Feedback link does not work.

I've tried both the pypi version and the latest master branch from gitlab.

I've tried cutting my docker image right back to remove possible clashes with other libraries

I can confirm that fetch_feedback works, and the files are pulled down; and I can confirm that the link works when in the classic interface - same docker image, just started with a different default_url value

From what I can see....

  • In classic mode, the link is ..../tree/<feedback_path>

  • In lab mode, the link has no attributes, but has an event link that runs

       function() {
        n.commands.execute("filebrowser:go-to-path", {
          path: i
        })
      }
    

and viewed from a javascript console, this calls ..../api/contents/<feedback_path> to get to ..../lab/tree/<feedback_path>

Can someone confirm that the View Feedback link works for them, in lab mode?

@perllaghu
Copy link
Contributor Author

(and to bring a note of levity.... can we call this The Kronenbourg Issue?

I'll get my coat...)

@brichet
Copy link
Contributor

brichet commented Sep 19, 2022

however the View Feedback link does not work

Thanks for reporting this. The link is the one in assignment list ?
Normally, it is supposed to change the path in the files browser panel on the left, to open the feedback directory (there is not change on the assignment list panel).
It works on my side, in a conda installation (not a docker).

@perllaghu
Copy link
Contributor Author

perllaghu commented Sep 19, 2022

Hmm...... I wonder if there's some proxy shenanigans going on.

Do you know of any other API calls that I can test to see if there's a general problem with my docker-in-k8s setup, or it's specifically this call?

.... hang on, I get Promethius metrics in Classic mode... so it's not specifically the url that's failing

@brichet
Copy link
Contributor

brichet commented Sep 20, 2022

I just came across this again, it looks like you already had this problem during the first tests of nbgrader on JupyterLab with your environment:
#1588 (comment)

A possible reason could be that your feedback path is not in the JupyterLab root path.
In your installation, is there JupyterHub and a service JupyterLab instance running (see https://nbgrader.readthedocs.io/en/stable/configuration/jupyterhub_config.html?highlight=service#example-use-case-one-class-multiple-graders) ?
If so, the assignment list may not be running on the correct server and may not have access to the feedback path.

@perllaghu
Copy link
Contributor Author

Indeed - I noticed it when the changes first came round.... then I had to go and finalise all the notebook offerings for our service in time for testing & rollout for the new academic year [with all the lead-time that requires] - so wasn't able to pursue properly at the time.

My feedback path is standard.... though with path_includes_course set true (so ~/<course_coce>/<assignment_code>/feedback/

There's no 'hub involved, we have a custom front-end that spawns into a k8s cluster directly.

If so, the assignment list may not be running on the correct server and may not have access to the feedback path.

This does not apply - the notebook server is complete & self-contained... using an external exchange to transfer assignment files between users rather than the default file-copy.

If I switch my 'lab UI to the classic interface - simply by editing the url from ..../lab to ..../tree - then the link becomes a fixed URL (as one would expect), and works fine.

In 'lab mode, I can traverse to the feedback - for example

.../lab/tree/Made up/20220916/feedback/2022-09-16 06%3A17%3A35.798776 UTC

however the event is calling

.../api/contents/Made up/20220916/feedback/2022-09-16%2006%3A17%3A35.798776%20UTC?content=0&1663667124469

Note that if I edit the event URL to be

.../lab/tree/Made up/20220916/feedback/2022-09-16%2006%3A17%3A35.798776%20UTC?content=0&1663667124469

then the UI sorts it self out & works just fine.

... so I'm deducing the api/contents bit is either failing due to containerization, or to the way we're handling things in the cluster.

Is there a reason why the call is to api/contents rather than lab/tree ?

@brichet
Copy link
Contributor

brichet commented Sep 20, 2022

Is there a reason why the call is to api/contents rather than lab/tree ?

For what I understand, both are calling the api/contents, but the classic opens a new tab in tree view while lab updates the filebrowser panel files. the lab/tree would be called to reload the full page.

When it calls the api/contents interface with the link, is there an error in the request? Is the status 200?

@perllaghu
Copy link
Contributor Author

Aha!!!

So - the fragment of html is created by code https://github.com/jupyter/nbgrader/blob/main/src/assignment_list/assignmentlist.ts#L444-L455

.... and I get an internal 404 - with the following in the javascript console:

filebrowser:go-to-path failed to go to: Made up/20220916/feedback/2022-09-16%2006%3A17%3A35.798776%20UTC Error: No such file or directory: Made up/20220916/feedback/2022-09-16%%2006%%3A17%%3A35.798776%%20UTC
    r https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    create https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    get https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    get https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    r https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    execute https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    execute https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    onclick https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    make_row https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    g https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    make_row https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    u https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    load_list_success https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    handle_load_list https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    load_list https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    change_course https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    load_list_success https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    handle_load_list https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    load_list https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    onclick https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    bind_events https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    v https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    _ https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    execute https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    execute https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    l https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    restore https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    restore https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    restore https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    activate https://noteable/user/123/lab/extensions/nbgrader/static/846.21d4664e162f1504d1eb.js?v=21d4664e162f1504d1eb:1
    promise https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    promise callback*80885/e.prototype.activatePlugin https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    o https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    start https://noteable/user/123/static/lab/jlab_core.8916587a095efe10064a.js?v=8916587a095efe10064a:2
    z https://noteable/user/123/static/lab/7796.a8997c070bf0dea0fb1a.js?v=a8997c070bf0dea0fb1a:1
    e https://noteable/user/123/static/lab/main.0225946bbbf4f4a6259a.js?v=0225946bbbf4f4a6259a:1
    37559 https://noteable/user/123/static/lab/main.0225946bbbf4f4a6259a.js?v=0225946bbbf4f4a6259a:1
    a https://noteable/user/123/static/lab/main.0225946bbbf4f4a6259a.js?v=0225946bbbf4f4a6259a:1
    <anonymous> https://noteable/user/123/static/lab/main.0225946bbbf4f4a6259a.js?v=0225946bbbf4f4a6259a:1
    <anonymous> https://noteable/user/123/static/lab/main.0225946bbbf4f4a6259a.js?v=0225946bbbf4f4a6259a:1

My initial thought that base_url isn't set.... however I can see the container was started with an arg --ServerApp.base_url=/user/123/ - so that should be correct!

@brichet
Copy link
Contributor

brichet commented Sep 20, 2022

Error: No such file or directory: Made up/20220916/feedback/2022-09-16%%2006%%3A17%%3A35.798776%%20UTC

Is it possible to reproduce the configuration without spaces in the directory names? It could be from there, maybe the URL is not formatted correctly (I think it should be Made%20up/20220916/...).

@perllaghu
Copy link
Contributor Author

Error: No such file or directory: Made up/20220916/feedback/2022-09-16%%2006%%3A17%%3A35.798776%%20UTC

Is it possible to reproduce the configuration without spaces in the directory names? It could be from there, maybe the URL is not formatted correctly (I think it should be Made%20up/20220916/...).

Yeah - I wondered about that too - however I've just confirmed with a course test and assignment foo - and I still get the same error.

@brichet
Copy link
Contributor

brichet commented Sep 21, 2022

I did some tests and I don't understand what's going on with the path, it look like it's partially formatted:
/api/contents/Made up/20220916/feedback/2022-09-16%2006%3A17%3A35.798776%20UTC?content=0&1663667124469

On my side:

  • if I print the path in the console (the one sent to go-to-path command), I get ./space name/feedback/2022-09-21 07:32:55.217797 UTC, not formatted at all.
  • the requested URL is api/contents/space%20name/feedback/2022-09-21%2007%3A32%3A55.217797%20UTC?content=1&1663746357643, fully formatted.

@perllaghu
Copy link
Contributor Author

Hmmm.... OK - I need to do something creative to build a "simple" stand-alone docker image to test... then move that into my cluster, then slowly expand it to my full image.

@perllaghu
Copy link
Contributor Author

So @brichet has a PR to fix the "demo build" stuff.... and made it available via Binder - and binder does, indeed, provide a working image.

However, the feedback link is still broken in that branch of code, as run on binder.

I've tested a number of browser/OS combinations, and none of them are doing the view feedback redirect as I would expect

Operating System Browser link works?
Linux Firefox No
Linux Chrome No
Linux Edge beta No
MacOS Firefox No
MacOS Chrome No
MacOS Safari No
Win 10 Home Firefox No
Win 10 Home Chrome No
Win 10 Home Edge beta No

I can confirm that the feedback files are actually pulled, by the student, into the right directory (~/ps1/feedback/<date & time>/.)

@brichet
Copy link
Contributor

brichet commented Sep 29, 2022

Right, in the case of Binder (and maybe in your configuration as well), there may be a confusion on paths. The feedback path is wrong from the root of the JupyterLab instance.
In the binder case, the feedback path starts with ./example_course/.. but the JupyterLab root_dir is already that directory : https://github.com/jupyter/nbgrader/blob/main/binder/jupyter_config.json

This feedback path come from server extension :

local_feedback_dir = os.path.join(
assignment_dir, 'feedback', info['timestamp'])

On Binder, the same behavior append for classic Notebook extension, the link is not working as well.

@brichet
Copy link
Contributor

brichet commented Sep 30, 2022

Thanks @perllaghu for all the testing.
I just opened #1675, which should fix the view feedback link in the Binder case.
The problem was due to the JupyterLab root_dir being different from ./.

@perllaghu
Copy link
Contributor Author

I just opened #1675, which should fix the view feedback link in the Binder case.

Once this PR is merged, this issue can be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants