Add Task timeout and params in Pipeline Desc

[sm43]

Changes

This adds Task's timeout and params with value in pipeline desc.
If param value is not defined checks for default value,
if default value is not defined, shows param type.
Closes #764 #765

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes tests (if functionality changed/added)
• Run the code checkers with make check
• Regenerate the manpages, docs and go formatting with make generated
• Commit messages follow commit message best practices

Release Notes

release-note

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Shivam Mukhade (23b676dca1b3476f7531eb92b56c7c097a3e6c03)

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/formatted/param.go	Do not exist	86.7%
pkg/formatted/time.go	0.0%	27.3%	27.3

[waveywaves]

/test pull-tekton-cli-build-tests

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/formatted/param.go	Do not exist	86.7%
pkg/formatted/time.go	0.0%	27.3%	27.3

[danielhelfand]

In testing this locally, I am seeing that running a tkn pipeline desc with a param with a default is not used for the param value:

🗒  Tasks

 NAME              TASKREF   TIMEOUT    RUNAFTER   PARAMS
 ∙ pipelinetask1   sleep1s   1 second              seconds: $(params.seconds)

[sm43]

In testing this locally, I am seeing that running a tkn pipeline desc with a param with a default is not used for the param value:

🗒  Tasks

 NAME              TASKREF   TIMEOUT    RUNAFTER   PARAMS
 ∙ pipelinetask1   sleep1s   1 second              seconds: $(params.seconds)

@danielhelfand I didn't get this. Could you please clarify, what is the expectation here in PARAMS?

[danielhelfand]

In testing this locally, I am seeing that running a tkn pipeline desc with a param with a default is not used for the param value:

🗒  Tasks

 NAME              TASKREF   TIMEOUT    RUNAFTER   PARAMS
 ∙ pipelinetask1   sleep1s   1 second              seconds: $(params.seconds)

@danielhelfand I didn't get this. Could you please clarify, what is the expectation here in PARAMS?

In thinking this over, I guess was expecting to see a default value specified for the param that is declared on the Pipeline. For example, if the following were declared on the pipeline, it would show the default specified if the param has no declared value.

params:
  - name: seconds
    default: 10s

However, I realize that's a bit of a difficult task. It just seems a bit odd to display the actual syntax of how the param is declared on the pipeline. I'm wondering if it would be maybe more useful to display the param type in this case?

[sm43]

In thinking this over, I guess was expecting to see a default value specified for the param that is declared on the Pipeline. For example, if the following were declared on the pipeline, it would show the default specified if the param has no declared value.

params:
  - name: seconds
    default: 10s

However, I realize that's a bit of a difficult task. It just seems a bit odd to display the actual syntax of how the param is declared on the pipeline. I'm wondering if it would be maybe more useful to display the param type in this case?

@danielhelfand I tried both default value and type.

 Params
 NAME           TYPE     DESCRIPTION   DEFAULT VALUE
 ∙ pl-param-x   string                 1
 ∙ pl-param-y   string                 1
🗒  Tasks
 NAME                TASKREF           TIMEOUT   RUNAFTER   PARAMS
 ∙ sum-params        sum-params        ---                  a: string, b: string
 ∙ multiply-params   multiply-params   ---                  a: string, b: string

and with default value

 NAME                TASKREF           TIMEOUT   RUNAFTER   PARAMS
 ∙ sum-params        sum-params        ---                  a: 1, b: 1
 ∙ multiply-params   multiply-params   ---                  a: 1, b: 1

which would be better to show? if value then do we need to mention that it is default value in bracket? And what if default value is not defined then?

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/formatted/param.go	Do not exist	95.7%
pkg/formatted/time.go	0.0%	27.3%	27.3

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/formatted/param.go	Do not exist	100.0%
pkg/formatted/time.go	0.0%	27.3%	27.3

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/formatted/param.go	Do not exist	100.0%
pkg/formatted/time.go	0.0%	27.3%	27.3

[danielhelfand]

which would be better to show? if value then do we need to mention that it is default value in bracket? And what if default value is not defined then?

Would be nice if the default value is shown if present but then show the type of no default value is available.

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/formatted/param.go	Do not exist	100.0%
pkg/formatted/time.go	0.0%	27.3%	27.3

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/formatted/param.go	Do not exist	100.0%
pkg/formatted/time.go	0.0%	27.3%	27.3

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/formatted/param.go	Do not exist	100.0%
pkg/formatted/time.go	0.0%	27.3%	27.3

[sm43]

Would be nice if the default value is shown if present but then show the type of no default value is available.

@danielhelfand yes. Now it checks for default value, if not available then shows param type in form null (param-type). I have changed the way ArrayVal is displayed. let me know if that's okay.

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/formatted/param.go	Do not exist	100.0%
pkg/formatted/time.go	0.0%	27.3%	27.3

[danielhelfand]

/lgtm

Thanks @sm43!

[sm43]

cc @piyush-garg @pradeepitm12

[vdemeester]

/lgtm

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 [vdemeester]

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