feat: override task timeouts in pipelineruns#8636
Conversation
|
/kind feature |
|
The following is the coverage report on the affected files.
|
41cbb8b to
1cb87f9
Compare
|
The following is the coverage report on the affected files.
|
1cb87f9 to
d5ca69c
Compare
|
The following is the coverage report on the affected files.
|
d5ca69c to
0418ce7
Compare
0418ce7 to
5b082b1
Compare
|
The following is the coverage report on the affected files.
|
f1a4a4a to
b10c867
Compare
|
The following is the coverage report on the affected files.
|
|
/retest |
aThorp96
left a comment
There was a problem hiding this comment.
/lgtm
One documentation suggestion to clarify the timeout assignment vs timeout enforcement behavior between TaskRuns and PipelineRuns. Otherwise this looks great. Thanks!
There was a problem hiding this comment.
Agreed, thanks for creating the followup issue as well! What you've added to that section looks good for the scope of this PR 🙏🏼
|
@aThorp96: changing LGTM is restricted to collaborators DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
The following is the coverage report on the affected files.
|
twoGiants
left a comment
There was a problem hiding this comment.
Looks great! The tests are very good. Thank you for the documentation update, it clarifies a lot. 😸 👍
I have found temporal coupling in the code. See my big comment below.
But all in all its good to go, it just needs some minor changes for a quick solution so it can be merged.
You could add a refactoring issue to centralize timeout validation for the community to pick up but its not a must.
| errs = errs.Also(apis.ErrInvalidValue( | ||
| fmt.Sprintf("%s should be <= %s %s", taskRunTimeout.Duration, timeoutSource, maxTimeout.Duration), | ||
| "timeout")) |
There was a problem hiding this comment.
| errs = errs.Also(apis.ErrInvalidValue( | |
| fmt.Sprintf("%s should be <= %s %s", taskRunTimeout.Duration, timeoutSource, maxTimeout.Duration), | |
| "timeout")) | |
| return apis.ErrInvalidValue( | |
| fmt.Sprintf("%s should be <= %s %s", taskRunTimeout.Duration, timeoutSource, maxTimeout.Duration), | |
| "timeout") |
c45c5af to
8aab142
Compare
|
The following is the coverage report on the affected files.
|
8aab142 to
075e4b6
Compare
Signed-off-by: Vibhav Bobade <[email protected]>
075e4b6 to
8679149
Compare
|
The following is the coverage report on the affected files.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aThorp96, twoGiants, vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
The following is the coverage report on the affected files.
|
|
/lgtm |
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
cornflakes > testflakes |
Changes
/fixes #7752
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes