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

Improved exception context #58

Merged
merged 6 commits into from
Oct 14, 2024
Merged

Improved exception context #58

merged 6 commits into from
Oct 14, 2024

Conversation

cpelley
Copy link
Collaborator

@cpelley cpelley commented Oct 3, 2024

Ensure that we can understand where an exception was actually raised.

Issues

@cpelley cpelley self-assigned this Oct 11, 2024
@cpelley cpelley added enhancement New feature or request test labels Oct 11, 2024
@cpelley cpelley marked this pull request as ready for review October 11, 2024 13:59
@SamGriffithsMO
Copy link

How does dask handle errors being raised currently?

I was thinking if we might want two separate cases:

  1. Operational - pool any raised errors, run as much as possible, then raise them at the end of the task
  2. Development - Raise errors as soon as they arise and end the task

@cpelley
Copy link
Collaborator Author

cpelley commented Oct 14, 2024

@SamGriffithsMO, indeed. I previously added the following fail_fast argument to the in-house scheduler to do just that (the in-house scheduler being what's used by EPP now).
Though I haven't yet explored with EPP what the best operational behaviour might be (we are far away from that yet). It could be that a fail_fast=True would be the more appropriate behaviour in both dev and oper scenarios now that we pass data in memory and intermediate steps are lost. That is, being able restart the task sooner could be the preferred choice in oper now - something to be decided.
Out of scope for this PR mind, but 👍 for raising.

Copy link

@SamGriffithsMO SamGriffithsMO left a comment

Choose a reason for hiding this comment

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

👍

@cpelley cpelley merged commit af7561a into main Oct 14, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants