Do not wrap workflow-failure exceptions from converters in workflows #882
Conversation
|
|
|
José A. Pastor seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
| @@ -1774,6 +1774,8 @@ def _convert_payloads( | |||
| # Don't wrap payload conversion errors that would fail the workflow | |||
| raise | |||
| except Exception as err: | |||
| if self._is_workflow_failure_exception(err): | |||
There was a problem hiding this comment.
Technically this has backwards compatibility concerns since it changes existing behavior based on raised exception, but I think it's ok in this instance.
But can we change the PR title to be a bit more generic so when it shows up in release notes it doesn't look like it's Pydantic specific? Maybe something like "do not wrap workflow-failure exceptions from converters in workflows"?
666ares
changed the title
|
@666ares - I am updating your branch with main and will merge if/when CI passes (can ignore some flakes there) |
What was changed
Fail workflow upon
pydantic.ValidationErrorwhen using thepydantic_data_converter, so that the user can pass that specific exception type toworkflow_failure_exception_typesin order for the workflow to fail instead of keeping it running, since there is no way that such a workflow will ever complete with an incorrect payload.Checklist
Closes [Feature Request] Fail workflow upon pydantic.ValidationError when using the pydantic_data_converter #815
How was this tested:
tests/worker/test_workflow::test_workflow_fail_on_bad_inputwas updated to check the right exception message.test/worker/test_workflow::test_workflow_fail_on_bad_pydantic_inputwas added to check that the workflow indeed fails when a Pydantic model can't be validated.Any docs updates needed?
No.