nbgrader icon indicating copy to clipboard operation
nbgrader copied to clipboard

Copy integrity

Open tuncbkose opened this issue 2 years ago • 5 comments

  • 11da69c: Adds error messages for when fetch/fetch_feedback fails
  • 783e0d2: Wraps copying function into try/except/finally to ensure that in the event of an error, already copied files have correct permissions
  • 8d8d898: Wraps copying function into try/except to ensure that even if not all files were copied, timestamp.txt is still created and feedback workflow doesn't break later
    • Right now, the student receives a "Submission Error" and no indication that something is submitted. However, files are still copied to the exchange folder.
    • I think it is fine to not prevent any copying (i.e. delete already-copied files if an error is encountered at any point), because if a student sees "Submission Error", they will likely try submitting again, hopefully after the problem is fixed.
    • The error message user sees could be clearer.

tuncbkose avatar Mar 15 '23 11:03 tuncbkose

Binder :point_left: Launch a Binder on branch AaltoSciComp/nbgrader/copy-2

github-actions[bot] avatar Mar 15 '23 11:03 github-actions[bot]

Thanks @tuncbkose.

  • 11da69c: Adds error messages for when fetch/fetch_feedback fails

Can you add tests on it please ?

  • For the lab part it should look like https://github.com/jupyter/nbgrader/blob/bf643e6b0e8e9fa2799c6a45e24891944448575d/nbgrader/tests/labextension_ui-tests/tests/test_assignment_list.spec.ts#L609
  • For the notebook part you can probably look at https://github.com/jupyter/nbgrader/blob/bf643e6b0e8e9fa2799c6a45e24891944448575d/nbgrader/tests/nbextensions/test_assignment_list.py#L446

For the others commit do you have a workflow how to reproduce the error ? Also, if it is possible, it would be wonderful to have tests on it.

brichet avatar Jun 12 '23 12:06 brichet

All of these came into play when files had wrong permissions (which was a concern for us, in case something might go wrong in mounting network drives). They are easily reproducible if you remove read rights from a notebook and try to submit it, for example.

I am not sure if I can do a similar thing for a test (or rather if it is a good idea), so I'll try to come up with something else. Let me know if you have any suggestions.

tuncbkose avatar Jun 12 '23 12:06 tuncbkose

I am slowly working on adding tests, but the following might be worth noting:

Currently, pressing the "fetch feedback" button when feedback is not available does nothing from the users' perspective. In my experience, this has led students to wonder if the button was working properly.

In the current state of the PR, pressing the button when feedback is not available will create an error dialogue with "Fetch Failed: feedback not fetched" and the traceback. Even though the traceback at the end includes "Assignment not found at ...", it may be unreasonable to expect students to infer from this that feedback is not available.

This change in my opinion is pretty neutral, but it might be worth changing the default error message to something like "Feedback not available or something went wrong".

tuncbkose avatar Jul 11 '23 08:07 tuncbkose

Is there a way to set some nbgrader configuration for a single test? Only https://github.com/jupyter/nbgrader/commit/783e0d25e5814c557414029a269447779b8faed8 remains untested, but it applies only when CourseDirectory.groupshared == true.

tuncbkose avatar Jul 13 '23 05:07 tuncbkose