Skip to content

fix(task): task run finished not consistent#304

Open
xibz wants to merge 1 commit intocdevents:mainfrom
xibz:consistency
Open

fix(task): task run finished not consistent#304
xibz wants to merge 1 commit intocdevents:mainfrom
xibz:consistency

Conversation

@xibz
Copy link
Copy Markdown
Contributor

@xibz xibz commented Mar 25, 2026

This commit task run to conform to other outcome values being enums with values: "success", "failure", "error", and "cancel"

This commit task run to conform to other outcome values being enums with
values: "success", "failure", "error", and "cancel"

Signed-off-by: xibz <impactbchang@gmail.com>
@xibz xibz requested a review from a team as a code owner March 25, 2026 20:37
"outcome": {
"type": "string"
"type": "string",
"enum": ["success", "failure", "cancel", "error"]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tests typically have assertions, which may succeed or fail, and in case the tests fails to execute before the assertion is evaluated, that's considered an error. How does that model apply to task runs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The same as how it applies to pipeline run. If pipeline run has it as enum, this should probably be an enum was my thinking

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, I see, it was added in #238.... well, I'm ok to make things consistent, but only if there's a meaning - I guess I already expressed by concerns at the time in #237.

I'm fine to keep the two type of failures for TaskRuns and PipelineRuns if there is a meaning to it.
For Tekton I wouldn't know which one to use. I guess we could use one for validation errors (so it didn't run at all) and another one for runtime errors, although the distinction is arbitrary as validation error, when using dynamic values in matrices and results, can also happen at runtime.

As longs as there's clear guidance for the meaning I'm ok. If not I would rather remove it from PipelineRuns.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, and that is a very important concern. The idea (my plan) is to add these to the _defs so it is very clear on what these mean. But holding off, until we define the subjects that encompass this. Ill create an issue for that, if that helps

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants