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

Enabled outputs for failed jobs #270

Merged
merged 4 commits into from
Nov 3, 2022
Merged

Conversation

3coins
Copy link
Collaborator

@3coins 3coins commented Nov 3, 2022

Summary

Failures for jobs can be of 2 types:

  1. Failures while creating the job, so no outputs are created.
  2. Failure while executing the notebooks due to an error within the notebook cell. These can generate outputs, but can still act as a failed job.

Current implementation doesn't allow for distinguishing between 1 and 2, so downloads for failed jobs that might have outputs are not enabled. This can lead to confusion by users, who don't have access to the failed notebook output or logs to understand where the problem existed.

Solution

  • Marked jobs with cell execution error as failed, ensured output files are written for this case.
  • Added error handling for downloader to ensure missing files doesn't block other file downloads.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Binder 👈 Launch a Binder on branch 3coins/jupyter-scheduler/failure-outputs

@3coins 3coins self-assigned this Nov 3, 2022
@3coins 3coins added the enhancement New feature or request label Nov 3, 2022
@ellisonbg
Copy link
Contributor

Can we avoid adding an extra status value by just allowing output files in the existing failure state?

@3coins
Copy link
Collaborator Author

3coins commented Nov 3, 2022

@ellisonbg and I discussed the current approach at length, and came to a conclusion that the backend doesn't have a way to know which files are actually written after a job execution (especially on failure). The add_job_files and get_staging_paths always point to a full set of files, not a subset based on the job status. Due to this, an implicit contract in current backend has emerged that requires all job files to be present regardless of the failed or success status.

Copy link
Collaborator

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jason already commented on frontend. The backend logic looks good.

@3coins 3coins merged commit 513fce3 into jupyter-server:main Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants