-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Eval diagnostics #26738
Eval diagnostics #26738
Conversation
This was mostly unused now, since we no longer needed to interrupt a series of eval node executions. The exception was the stopHook, which is still used to halt execution when there's an interrupt. Since interrupting execution should not complete successfully, we use a normal opaque error to halt everything, and return it to the UI. We can work on coalescing or hiding these if necessary in a separate PR.
Codecov Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Geez @jbardin who opens PRs like this, it's way too big (❤️)
I really like this! The terraform
package gets more readable with every pass, AND you took care of something I'd been hoping to get to 🎉
FWIW, I'm generally positive towards "being cancelled is an error", and consider that to be better behavior even if it's not what it used to be doing. 🤷♂️ |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
This completes the diagnostics portion of the Eval node refactor. Diagnostics are now maintained from the RPC calls all the way through core error handling. The most noticeable external benefit to this is that warnings from plugins can now be passed all the way through to the UI even when there are no accompanying errors for them to piggy-back on.
We do have a change in behavior to note however. Not using raw errors during eval makes it much harder to hide the error that was being used to stop execution when there's an interrupt. This however may not be an issue, since a partial execution should not result in a 0 exit code, and most unix process in general exit with a non-0 code when interrupted. Merging this early in the 0.15 development cycle gives us time to evaluate the change, and assess if we need to work on another method to hide that error.
Also closes #26680