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

Show condition run as child for taskrun #292

Merged
merged 6 commits into from
May 25, 2020
Merged

Conversation

evidolob
Copy link
Collaborator

As side effect of this PR, tree load for PipelineRun and it child become more faster,
now we use only PipelineRun data to build subtree for pipelinerun,
previously we fetch TaskRun's for each PipelineRun

Example:
Screenshot 2020-05-20 at 12 09 55

Fix: #277

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
@evidolob evidolob self-assigned this May 20, 2020
Copy link
Contributor

@sudhirverma sudhirverma left a comment

Choose a reason for hiding this comment

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

I am getting an error while starting a condition pipeline

Screenshot 2020-05-20 at 5 35 45 PM

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
@evidolob
Copy link
Collaborator Author

evidolob commented May 21, 2020

Current state demo:
ezgif com-video-to-gif (12)

@siamaksade I had to introduce new state icon for task in pipelinerun, which indicates that task is pending, and it will show icon for canceled task(we already have it) WDYT?

And another question that with this PR, it is possible to click on 'Open in editor' icon for task, which always throw error until task is not started. As we relay on information provided by pipelinerun json/yaml, which contains all commands, but corresponding TaskRun will not appear until task start running.

@codecov-commenter
Copy link

codecov-commenter commented May 21, 2020

Codecov Report

Merging #292 into master will decrease coverage by 0.67%.
The diff coverage is 43.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #292      +/-   ##
==========================================
- Coverage   74.12%   73.44%   -0.68%     
==========================================
  Files          44       44              
  Lines        3142     3216      +74     
  Branches      565      583      +18     
==========================================
+ Hits         2329     2362      +33     
- Misses        813      854      +41     
Impacted Files Coverage Δ
src/tekton/tektonitem.ts 86.66% <0.00%> (-4.83%) ⬇️
src/tkn.ts 65.10% <41.86%> (-4.28%) ⬇️
src/pipeline/pipeline-graph.ts 66.37% <50.00%> (+8.79%) ⬆️
src/pipeline/pipelineExplorer.ts 65.90% <100.00%> (+1.62%) ⬆️
src/util/tekton-vfs.ts 96.34% <100.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 651ed2d...fa11ea0. Read the comment docs.

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
@siamaksade
Copy link

@evidolob the pending icon looks good, and better representative of that state of pipelinerun. Could we also use a different icon for the condition?

About "Open in Editor", showing and error is fine. The error message should explain that the taskrun is not started yet and user can open it in the editor as soon as it starts running.

@evidolob
Copy link
Collaborator Author

@siamaksade Yes we can, I'm not sure which one to use. As in general condition is also taskrun, and may have same states as regular taskrun.

…to condition-as-child

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
@evidolob
Copy link
Collaborator Author

evidolob commented May 21, 2020

I think we can use our icons for other resources(with abbreviation ) and change colors of it depending on state(success/fail/pending/canceled)

Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
@evidolob
Copy link
Collaborator Author

Latest state demo:
ezgif com-video-to-gif (13)

@evidolob evidolob merged commit cd33f57 into master May 25, 2020
@evidolob evidolob deleted the condition-as-child branch May 25, 2020 11:29
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.

Use condition name for 'Condition' node in pipeline run child nodes
4 participants