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.

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

Change ClusterTask Timeout from Int to String and Remove -t Shorthand #784

Closed
danielhelfand opened this issue Mar 6, 2020 · 6 comments
Closed
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.
Milestone

Comments

@danielhelfand
Copy link
Member

danielhelfand commented Mar 6, 2020

Similar to #779, tkn clustertask start --timeout should be changed to a string duration format (i.e. 1h30m2s). Currently it is an integer that is used to specify a timeout in seconds.

Similarly, the -t shorthand should be removed to keep consistency across all timeout commands.

Unfortunately, this was missed in our previous round of deprecation/behavior change notices around timeout, so this change cannot take place until v1.0.0. We should log a warning message to users about this change in version v0.9.0. It can be the exact same format as what was used in #753.

v0.9.0 Changes

Log warning message to user whenever --timeout is used with tkn ct start:

WARNING: The --timeout flag will no longer be specified in seconds in v1.0.0. Learn more here: https://github.com/tektoncd/cli/issues/784
WARNING: The -t shortand for --timeout will no longer be available in v1.0.0.

v1.0.0 Changes

The overall goal here will be to specify a Timeout via tkn as follows:

tkn clustertask start ctName --timeout 30m2s
@danielhelfand danielhelfand added this to the 1.0.0 🦁 milestone Mar 6, 2020
@danielhelfand danielhelfand added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Mar 6, 2020
@danielhelfand
Copy link
Member Author

/assign @kadern0

@tekton-robot
Copy link
Contributor

@danielhelfand: GitHub didn't allow me to assign the following users: kadern0.

Note that only tektoncd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @kadern0

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.

@danielhelfand
Copy link
Member Author

@kadern0, I think you have to assign yourself to the issue.

@kadern0
Copy link
Contributor

kadern0 commented Mar 11, 2020 via email

@vdemeester
Copy link
Member

/assign @kadern0

(this works as soon as you commented once, I think)

@vdemeester vdemeester modified the milestones: 1.0.0 🦁, 0.10.0 🐅 Apr 22, 2020
kadern0 added a commit to kadern0/cli that referenced this issue Apr 30, 2020
Fixes issue tektoncd#784

Signed-off-by: Pablo Caderno <kaderno@gmail.com>
tekton-robot pushed a commit that referenced this issue Apr 30, 2020
Fixes issue #784

Signed-off-by: Pablo Caderno <kaderno@gmail.com>
@danielhelfand
Copy link
Member Author

Closed by #793

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.
Projects
None yet
Development

No branches or pull requests

4 participants