Skip to content

Fix error message not being embedded if step is Status.error#98

Merged
modehnal merged 1 commit intomainfrom
fp/error_handling
Sep 3, 2025
Merged

Fix error message not being embedded if step is Status.error#98
modehnal merged 1 commit intomainfrom
fp/error_handling

Conversation

@fpokryvk
Copy link
Copy Markdown
Collaborator

@fpokryvk fpokryvk commented Sep 3, 2025

This deserves better rewrite. We should not check status == Status.failed but rather status in failed_statuses or status not in success_statuses. But that is for another rewrite, hotfixing by replacing error to failed for now.

This deserves better rewrite. We should not check `status == Status.failed` but rather `status in failed_statuses` or `status not in success_statuses`. But that is for another rewrite, hotfixing by replacing error to failed for now.
@fpokryvk fpokryvk requested a review from modehnal September 3, 2025 07:30
Copy link
Copy Markdown
Collaborator

@modehnal modehnal left a comment

Choose a reason for hiding this comment

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

LGTM

@modehnal modehnal merged commit e4b2040 into main Sep 3, 2025
25 checks passed
@jenisys
Copy link
Copy Markdown
Member

jenisys commented Sep 3, 2025

Mmh, I am just curious:
Why are not using the Status.has_failed() method that covers the various failure/error/hook-error cases ?

SEE ALSO:

@modehnal
Copy link
Copy Markdown
Collaborator

modehnal commented Sep 3, 2025

We were not aware of that method, but were thinking about a way to do just that. Thank you. In next update we will do it with this method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants