berkeley-dsep-infra / datahub

JupyterHubs for use by Berkeley enrolled students
https://docs.datahub.berkeley.edu
BSD 3-Clause "New" or "Revised" License
65 stars 39 forks source link

Empty notebooks submitted to okpy / otter #2107

Open yuvipanda opened 3 years ago

yuvipanda commented 3 years ago

For a while, there have been problems where okpy's submit notebook functionality was saving blank notebooks. Just before submitting, it injects some JS to save the notebook. But somehow, notebook saves sometimes take >10s and possibly fail. okpy then submits a blank notebook, and folks possibly lose work.

This seems to happen only to some folks, and has been hard to debug. From investigation so far, this has no obvious correlations with:

  1. Node the pod is on
  2. NFS machine health / stats

This doesn't seem to happen to anyone other than data8 students.

In the meantime, the data8 folks have added a retry to ok.submit() to make sure it works.

yuvipanda commented 3 years ago

1746 is related in some form.

samwu101 commented 3 years ago

Here's the aforementioned temporary fix (which isn't actually actively retrying but is just waiting longer so that the save will hopefully eventually happen) that @Yanay1 wrote.

from client.api.notebook import *
def new_save_notebook(self):
    """ Saves the current notebook by
        injecting JavaScript to save to .ipynb file.
    """
    try:
        from IPython.display import display, Javascript
    except ImportError:
        log.warning("Could not import IPython Display Function")
        print("Make sure to save your notebook before sending it to OK!")
        return

    if self.mode == "jupyter":
        display(Javascript('IPython.notebook.save_checkpoint();'))
        display(Javascript('IPython.notebook.save_notebook();'))
    elif self.mode == "jupyterlab":
        display(Javascript('document.querySelector(\'[data-command="docmanager:save"]\').click();'))   

    print('Saving notebook...', end=' ')

    ipynbs = [path for path in self.assignment.src
              if os.path.splitext(path)[1] == '.ipynb']
    # Wait for first .ipynb to save
    if ipynbs:
        if wait_for_save(ipynbs[0]):
            print("Saved '{}'.".format(ipynbs[0]))
        else:
            log.warning("Timed out waiting for IPython save")
            print("Could not automatically save \'{}\'".format(ipynbs[0]))
            print("Make sure your notebook"
                  " is correctly named and saved before submitting to OK!".format(ipynbs[0]))
            return False                
    else:
        print("No valid file sources found")
    return True

def wait_for_save(filename, timeout=600):
    """Waits for FILENAME to update, waiting up to TIMEOUT seconds.
    Returns True if a save was detected, and False otherwise.
    """
    modification_time = os.path.getmtime(filename)
    start_time = time.time()
    while time.time() < start_time + timeout:
        if (os.path.getmtime(filename) > modification_time and
            os.path.getsize(filename) > 0):
            return True
        time.sleep(0.2)
    print("\nERROR!\n YOUR SUBMISSION DID NOT GO THROUGH."\
        " PLEASE TRY AGAIN. IF THIS PROBLEM PERSISTS POST ON"\
        " PIAZZA RIGHT AWAY.\n ERROR!" + "\n"*20)
    return False

Notebook.save_notebook = new_save_notebook
samwu101 commented 3 years ago

Also, I want to clarify that the notebook isn't actually empty, but rather the Okpy submission is empty (i.e. doesn't contain a .ipynb file at all).

yuvipanda commented 3 years ago

okpy is no longer used. Is this still an issue, @samwu101?

samwu101 commented 3 years ago

I graduated and left the grading team right before our auto-grading system was revamped, so I'm not completely sure even though I would think this is no longer an issue. @Yanay1 or someone who worked on the grading team during the summer could probably answer this question.

Yanay1 commented 3 years ago

I think students still occasionally see issues with long wait times for their notebooks to save, which affects otter grader as well. (I’ve also graduated).

yuvipanda commented 3 years ago

@davidwagner reports this is still happening with data8 and otter as well. We did a tiny bit of investigation, and filed the following issues / next actions:

Particularly, @davidwagner is trying to find someone who can take on https://github.com/ucbds-infra/otter-grader/issues/450, which I think will help. I also opened https://github.com/ucbds-infra/otter-grader/pull/447 which @chrispyles merged, which might also help.

Yanay1 commented 3 years ago

When this issue happened for Okpy I increased the time out to around 10 minutes and it still ran into the issue for some students (a datahub issue), but it did reduce it in general from what I remember.

One solution would be to notify students if the timeout occurs.

Yanay1 commented 3 years ago

You can actually probably copy a lot of the same code I used for Okpy solution that Sam posted above (the wait for save function)!