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.

Sign up for GitHub

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

Jump to bottom

Deprecate timeout and promote timeouts in PR #4813

Merged
merged 1 commit into from
May 12, 2022
Merged

Deprecate timeout and promote timeouts in PR #4813

merged 1 commit into from
May 12, 2022

Conversation

vinamra28
Copy link
Member

Changes

With TEP-0046 a combination of timeouts was proposed as

timeouts:
  pipeline: 30m
  task: 30m
  finally: 30m

The following was introduced as alpha feature and it's been almost a while and now we can safely move that out of alpha gate.

This PR promotes the combined timeouts from alpha to stable and also deprecation existing timeout which can only be used to apply timeout at Pipeline level.

Signed-off-by: vinamra28 jvinamra776@gmail.com

/kind feature

closes #4460

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Docs included if any changes are user facing
  • Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Release notes block below has been filled in
    (if there are no user facing changes, use release note "NONE")

Release Notes

Deprecate timeout field in PipelineRuns and promote PipelineRun.Timeouts field from alpha to stable

@tekton-robot tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels May 1, 2022
@tekton-robot tekton-robot requested review from afrittoli and jerop May 1, 2022 09:51
@tekton-robot tekton-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 1, 2022
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 98.8% 98.8% -0.0

@tekton-robot tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 1, 2022
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 98.8% 98.8% -0.0

lbernick
lbernick reviewed May 2, 2022
View reviewed changes
Copy link
Member

@lbernick lbernick left a comment

Choose a reason for hiding this comment

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

LGTM! Let me know when this is ready for review.

docs/deprecations.md Outdated Show resolved Hide resolved
@jerop jerop added this to the Pipelines v0.36 milestone May 3, 2022
@vinamra28 vinamra28 force-pushed the pr-timeouts branch 2 times, most recently from 394c6a5 to 28854a4 Compare May 8, 2022 07:58
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 98.8% 98.8% -0.0

@vinamra28 vinamra28 marked this pull request as ready for review May 8, 2022 08:41
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 8, 2022
@vinamra28 vinamra28 requested a review from lbernick May 8, 2022 08:41
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 98.8% 98.8% -0.0

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 98.8% 98.8% -0.0

@tekton-robot tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 8, 2022
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 98.8% 98.8% -0.0

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 98.8% 100.0% 1.2

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 98.8% 100.0% 1.2

@vinamra28
Copy link
Member Author

/retest

1 similar comment
@vinamra28
Copy link
Member Author

/retest

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 10, 2022
@vinamra28
Copy link
Member Author

/retest

@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 98.8% 100.0% 1.2

@vinamra28
Deprecate timeout and promote timeouts in PR
5aef0ea 
With TEP-0046 a combination of timeouts was proposed as

timeouts:
  pipeline: 30m
  task: 30m
  finally: 30m
The following was introduced as alpha feature and it's been
almost a while and now we can safely move that out of alpha gate.

This commit promotes the combined timeouts from alpha to stable
and also deprecation existing timeout which can only be
used to apply timeout at Pipeline level.

Signed-off-by: vinamra28 jvinamra776@gmail.com
@tekton-robot
Copy link
Collaborator

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go 98.8% 100.0% 1.2

@vinamra28
Copy link
Member Author

/retest

@vinamra28
Copy link
Member Author

/cc @vdemeester @jerop @pritidesai

vdemeester
vdemeester approved these changes May 12, 2022
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

/lgtm
/meow

@tekton-robot
Copy link
Collaborator

@vdemeester: cat image

In response to this:

/lgtm
/meow

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.

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 12, 2022
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick, vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [lbernick,vdemeester]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot merged commit 84229fc into tektoncd:main May 12, 2022
jerop added a commit to jerop/pipeline that referenced this pull request Jun 11, 2022
@jerop
Remove PipelineRun timeouts from alpha features
1125bfc 
`PipelineRun` timeouts were promoted to beta in this pull request:
tektoncd#4813.

In this change, we update the alpha features table to remove
`PipelineRun` timeouts.

Related issue: tektoncd#4460.
jerop added a commit to jerop/pipeline that referenced this pull request Jun 16, 2022
@jerop
PipelineRun timeouts is in Beta
ccd47b2 
`PipelineRun` timeouts were promoted to Beta. This change removes
the listing of `PipelineRun` timeouts from the alpha features.

Issue: tektoncd#4460
PR: tektoncd#4813
@jerop jerop mentioned this pull request Jun 16, 2022
Merged
3 tasks
jerop added a commit to jerop/pipeline that referenced this pull request Jun 16, 2022
@jerop
PipelineRun timeouts is in Beta
968d84f 
`PipelineRun` timeouts were promoted to Beta. This change removes
the listing of `PipelineRun` timeouts from the alpha features.

Issue: tektoncd#4460
PR: tektoncd#4813
tekton-robot pushed a commit that referenced this pull request Jun 17, 2022
@jerop @tekton-robot
PipelineRun timeouts is in Beta
fc1f611 
`PipelineRun` timeouts were promoted to Beta. This change removes
the listing of `PipelineRun` timeouts from the alpha features.

Issue: #4460
PR: #4813
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lbernick lbernick lbernick approved these changes

@vdemeester vdemeester vdemeester approved these changes

@afrittoli afrittoli Awaiting requested review from afrittoli

@jerop jerop Awaiting requested review from jerop

@pritidesai pritidesai Awaiting requested review from pritidesai

Assignees

@vdemeester vdemeester

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
Pipelines v0.36
Development

Successfully merging this pull request may close these issues.

PipelineRun Timeouts
5 participants
@vinamra28 @tekton-robot @vdemeester @lbernick @jerop