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
Refactor built-in plans to use contingency_wrapper #1288
Comments
Reviewers: @danielballan @tacaswell |
should mark
|
files that mention
When I'm looking atthe I am not sure where the change is to be made. |
I'm weakly against deprecating it. In cases where you only want to do cleanup the simpler signature is nice. I'm also sure we have some usage on the floor at facilities who have already deployed bluesky and the disruption of deprecating this does not seem worth the marginal gain of getting rid of name in the namespace. On the other hand "pause for debug" was a bad idea (I think I wrote it). Maybe we should only purge it from the narrative docs? |
Not seeing any current use of |
should the issue be closed as not needed then? |
The action for this issue might be to ensure the documentation for the various affected code contains the recommendation in the initial statement here:
|
@stan-dot -- Thanks for picking this one up! |
no worries! doing a PR for the recommedation for the edit now |
The
contingency_wrapper
is better than thefinalize_wrapper
because the "final plan" takes in the exception that was raised as the argument, which allows the clean up to target specific failure modes if desired.Even where that capability isn't used, it would be an improvement I think to model using
contingency_wrapper
in our code so that users can easily discover it and find examples of its use.The text was updated successfully, but these errors were encountered: