search

jupyter / nbgrader

A system for assigning and grading notebooks
https://nbgrader.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
1.27k stars 317 forks source link

Release feedback fails after #1774 #1897

Open iyanmv opened 1 month ago

iyanmv commented 1 month ago

I think because of #1774, trying to re-release feedback can fail if submission was done prior to the update because the current code does not take into account that the file submission_secret.txt might not exist.

https://github.com/jupyter/nbgrader/blob/e0b2e65a56d483560f81a02c8a0b323f4659b10d/nbgrader/exchange/default/release_feedback.py#L66-L67

Operating system

nbgrader --version

Python version 3.11.8 | packaged by conda-forge | (main, Feb 16 2024, 20:53:32) [GCC 12.3.0]
nbgrader version 0.9.3

jupyterhub --version (if used with JupyterHub)

4.0.2

jupyter notebook --version

7.2.1

Expected behavior

Updating nbgrader should not break releasing feedback of older submissions.

Actual behavior

If submission_secret.txt does not exist in the feedback folder, release feedback will fail.

Steps to reproduce the behavior

  1. Take an old submission (from before updating to current version)
  2. Recreate feedback (this works)
  3. Release feedback (this fails)
tuncbkose commented 1 month ago

Oops, I guess at the time of making that PR, I didn't think that people would upgrade nbgrader in the middle of a course, which is my fault.

As a quick fix, it may be possible to downgrade and release feedback for that student individually (so that other submissions don't create problems if they have a submission_secret.txt).

For a more general fix, I guess the old code is still there, so we could have an if statement seeing if the file exists. But suppose a case where the submission_secret.txt was created then deleted for some reason. Since something went wrong, I would argue nbgrader should complain, and this solution could prevent that. So I'd be happier with marking this as a breaking change instead.

Happy to hear other opinions as well!

iyanmv commented 1 month ago

Hi! Thanks for the quick reply. It's not a big problem for us, but I just wanted to let you know because I guess it was an unintended side effect of that PR.

And yes, we recreate our JupyterHub image every week updating all packages (nbgrader included) using the compatible release operator. In the case of nbgrader I use nbgrader~=0.9. I guess such a change would have been better to keep it for 0.10.x. But if no one else reported it, I guess most people fix the version completely and only update before starting a new course.