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

Guard calling after_transition when exception encountered #7156

Merged
merged 5 commits into from
Oct 14, 2022

Conversation

zangell44
Copy link
Collaborator

This PR adds logic to guard against calling after_transition if orchestration encountered an error.

This applies in cases where, for example, we could encountered in error trying to secure concurrency slots in before_transition because of a connectivity issue with the database. Often exceptions of this sort will impact our ability to communicate with the database further, resulting in mishandled errors.

Closes https://github.com/PrefectHQ/nebula/issues/1041

Example

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • This pull request includes tests or only affects documentation.
  • This pull request includes a label categorizing the change e.g. fix, feature, enhancement

@zangell44 zangell44 added the fix A fix for a bug in an existing feature label Oct 13, 2022
@netlify
Copy link

netlify bot commented Oct 13, 2022

Deploy Preview for prefect-orion ready!

Name Link
🔨 Latest commit ef9d62f
🔍 Latest deploy log https://app.netlify.com/sites/prefect-orion/deploys/634969d505cbad000856f2c9
😎 Deploy Preview https://deploy-preview-7156--prefect-orion.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@zanieb
Copy link
Contributor

zanieb commented Oct 13, 2022

I thought this was intentional to ensure that after_transition could be used to clean up side-effects on failure? cc @anticorrelator

I have been informed that this is not the case :)

Gosh "errorred" is ugly to read though. Can we just do "transition_error" or "transition_exception" or "exception_in_transition"?

Copy link
Contributor

@zanieb zanieb left a comment

Choose a reason for hiding this comment

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

LGTM

@zangell44 zangell44 merged commit 2234b86 into main Oct 14, 2022
@zangell44 zangell44 deleted the orchestration-rules-robust-to-errors branch October 14, 2022 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A fix for a bug in an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants