Update TaskRun List Page Columns & PipelineRun Tab #6840

debsmita1 · 2020-10-05T18:29:51Z

JIRA story:
https://issues.redhat.com/browse/ODC-4800

Solution Description:

added Task Runs tab in the pipelineRun details page which lists all the Task Runs associated with the Pipeline Run
Added columns pipeline , task & pod & removed duration
The pipeline column is not shown in the TaskRun Tab under Pipeline Run details page
fixed the TaskRun list in the Search page
added tests for TaskRunsRow

GIF/Screenshots:

Test Coverage:

debsmita1 · 2020-10-05T18:30:12Z

/kind feature

debsmita1 · 2020-10-05T18:31:08Z

/cc @andrewballantyne

andrewballantyne · 2020-10-05T18:56:05Z

frontend/packages/dev-console/src/components/taskruns/list-page/TaskRunsHeader.tsx

@@ -10,33 +10,39 @@ const TaskRunsHeader = () => {
      props: { className: tableColumnClasses[0] },
    },
    {
-      title: 'Namespace',


Can you really remove Namespace? I think if you go to the Admin tab page and do all projects, you'll be missing this column, no?

@andrewballantyne Thank you for pointing this out. I noticed in the pipelines list page that the namespace column shows up when the user is not in the context of a project. I have incorporated that in my PR and have added a GIF in PR description

debsmita1 · 2020-10-06T11:11:42Z

/cc @bgliwa01

abhinandan13jan · 2020-10-06T12:41:30Z

frontend/packages/dev-console/src/components/taskruns/list-page/TaskRunsListPage.tsx

 import TaskRunsList from './TaskRunsList';
 import { TaskRunModel } from '../../../models';
 import { runFilters as taskRunFilters } from '../../pipelines/detail-page-tabs/PipelineRuns';

 interface TaskRunsListPageProps {
  hideBadge?: boolean;
+  showCreateButton?: boolean;


Why do we need this? Is it passed by a parent component somewhere?

karthikjeeyar · 2020-10-06T13:24:42Z

frontend/packages/dev-console/src/components/pipelineruns/detail-page-tabs/TaskRuns.tsx

+const TaskRuns: React.FC = (props) => (
+  <TaskRunsListPage
+    showTitle={false}
+    selector={{ 'tekton.dev/pipelineRun': _.get(props, 'obj.metadata.name') }}


we could replace the lodash _.get with optional chaining and remove the lodash import. why to import for this simple usage, WDYT?

karthikjeeyar · 2020-10-06T13:59:40Z

frontend/packages/dev-console/src/components/pipelineruns/PipelineRunDetailsPage.tsx

@@ -24,6 +25,11 @@ const PipelineRunDetailsPage: React.FC<DetailsPageProps> = (props) => {
      pages={[
        navFactory.details(PipelineRunDetails),
        navFactory.editYaml(viewYamlComponent),
+        {
+          href: 'TaskRuns',


Please change this to lower case hyphenated words in the url, so that we are using similar url segments in both the perspectives.

Suggested change

href: 'TaskRuns',

href: 'task-runs',

Odd, the Pipeline Details Page has a "Runs" Tab for Pipeline Runs 😕

Definitely agree we should kebab-case the route names though.

bgliwa01 · 2020-10-08T13:24:09Z

Is there a tooltip on the failed status showing the error?

bgliwa01 · 2020-10-08T13:26:24Z

will there be a separate PR adding a task run details page?

debsmita1 · 2020-10-08T15:09:05Z

will there be a separate PR adding a task run details page?

@bgliwa01 yes this is the PR #6851

Is there a tooltip on the failed status showing the error?

No, there isn't

andrewballantyne · 2020-10-08T15:16:44Z

Is there a tooltip on the failed status showing the error?

No, there isn't

@bgliwa01 I don't think we have a convention for showing a tooltip on any status 🤔 Can you point me at a case where we do this?

bgliwa01 · 2020-10-08T15:22:28Z

@andrewballantyne the only reason I'm asking is because it's in the ACs. I can't get a pipeline run to fail to validate if we have it or not currently.

bgliwa01 · 2020-10-08T15:26:51Z

Just verified that we don't show error in tooltip...I will ask PM

andrewballantyne · 2020-10-08T15:43:59Z

@andrewballantyne the only reason I'm asking is because it's in the ACs. I can't get a pipeline run to fail to validate if we have it or not currently.

Damn, good catch @bgliwa01! I did not at all see that when I was validating the AC. I guess I saw "status" and assumed "well that's pretty standard". I have asked a question on the Epic.

jerolimov

@debsmita1 could we also change the default order of TaskRuns to the "started" date (newest first), similar to PipelineRuns?

Imo this makes sense in the overall search list as well in the PipelineRun > TaskRuns tab.

edit: Hmmm, but we have then in the Pipeline Run > TaskRuns the inverted order then the logs tab...

Tested it locally, everything else works as expected. 👍

andrewballantyne · 2020-10-09T18:37:24Z

frontend/packages/dev-console/src/components/taskruns/list-page/TaskRunsRow.tsx

+        )}
+      </TableData>
+      <TableData className={tableColumnClasses[4]}>
+        {obj.status.podName ? (


I got a NPE on this line. status is optional by the K8sResourceKind type -- which happens to be all that the TaskRun is right now.

andrewballantyne · 2020-10-09T18:40:16Z

frontend/packages/dev-console/src/components/taskruns/list-page/TaskRunsRow.tsx

      </TableData>
      <TableData className={tableColumnClasses[3]}>
-        <Timestamp timestamp={obj?.status?.startTime} />
+        {obj.spec.taskRef?.name ? (


We need to define the TaskRunKind type before we start digging into spec. Now that this is an officially supported page we should definitely try to expand our types to handle these properties.

I have expanded the TaskRunKind in this PR #6867

andrewballantyne · 2020-10-09T18:42:12Z

frontend/packages/dev-console/src/components/taskruns/list-page/TaskRunsRow.tsx


 const TaskRunsRow: RowFunction<TaskRunKind> = ({ obj, index, key, style }) => {
  const taskRunsReference = referenceForModel(TaskRunModel);
+  const taskReference = referenceForModel(TaskModel);
+  const pipelineReference = referenceForModel(PipelineModel);


🤔 These are static references, I wonder if we don't just want to define them outside of the TaskRunsRow component. They don't ever change.

moved them oustide of the TaskRunsRowcomponent

andrewballantyne · 2020-10-09T18:43:24Z

frontend/packages/dev-console/src/components/pipelineruns/detail-page-tabs/TaskRuns.tsx

+import { K8sResourceKind } from '@console/internal/module/k8s';
+
+interface TaskRunsProps {
+  obj: K8sResourceKind;


Suggested change

obj: K8sResourceKind;

obj: TaskRunKind;

andrewballantyne · 2020-10-09T19:07:47Z

frontend/packages/dev-console/src/components/taskruns/list-page/TaskRunsListPage.tsx

  return (
    <ListPage
      {...props}
-      canCreate={false}
+      canCreate={kind?.includes(referenceForModel(TaskRunModel)) ?? false}


Curious, why this change?

this adds the create task run button in the search page when the Task Runs resource type is selected

bgliwa01

confirmed with PM that tooltip on error will be taken out of ACs for this epic...lgtm!!

karthikjeeyar · 2020-10-13T15:25:33Z

Verified locally, works fine
/lgtm

debsmita1 · 2020-10-15T10:23:28Z

/retest

debsmita1 · 2020-10-15T11:32:37Z

/retest

karthikjeeyar · 2020-10-15T16:02:00Z

Verified locally, works fine
/lgtm

bgliwa01

lgtm

rohitkrai03

/approve

openshift-ci-robot · 2020-10-19T11:04:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bgliwa01, debsmita1, karthikjeeyar, rohitkrai03

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:

~~frontend/packages/dev-console/OWNERS~~ [rohitkrai03]

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

debsmita1 · 2020-10-19T13:41:09Z

/test e2e-gcp-console

debsmita1 · 2020-10-19T15:33:07Z

/retest

openshift-bot · 2020-10-19T20:53:28Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-19T21:11:10Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-19T21:32:26Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-10-19T21:45:30Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 5, 2020

debsmita1 changed the title ~~{WIP] Update TaskRun List Page Columns & PipelineRun Tab~~ [WIP] Update TaskRun List Page Columns & PipelineRun Tab Oct 5, 2020

openshift-ci-robot added the component/dev-console Related to dev-console label Oct 5, 2020

openshift-ci-robot requested review from abhi-kn and jerolimov October 5, 2020 18:30

openshift-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 5, 2020

openshift-ci-robot requested a review from andrewballantyne October 5, 2020 18:31

andrewballantyne reviewed Oct 5, 2020

View reviewed changes

debsmita1 force-pushed the taskrun-list-page branch from 8a8f8ba to 75a7be7 Compare October 6, 2020 10:40

debsmita1 changed the title ~~[WIP] Update TaskRun List Page Columns & PipelineRun Tab~~ Update TaskRun List Page Columns & PipelineRun Tab Oct 6, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 6, 2020

debsmita1 requested a review from andrewballantyne October 6, 2020 10:48

openshift-ci-robot requested a review from bgliwa01 October 6, 2020 11:11

abhinandan13jan reviewed Oct 6, 2020

View reviewed changes

karthikjeeyar suggested changes Oct 6, 2020

View reviewed changes

debsmita1 force-pushed the taskrun-list-page branch from 75a7be7 to 236a58c Compare October 7, 2020 08:04

debsmita1 requested review from karthikjeeyar and abhinandan13jan October 7, 2020 18:42

jerolimov suggested changes Oct 9, 2020

View reviewed changes

andrewballantyne suggested changes Oct 9, 2020

View reviewed changes

debsmita1 requested review from jerolimov and andrewballantyne October 12, 2020 09:13

bgliwa01 approved these changes Oct 12, 2020

View reviewed changes

openshift-ci-robot assigned karthikjeeyar Oct 13, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2020

bgliwa01 mentioned this pull request Oct 14, 2020

TaskRun details and log page #6851

Merged

debsmita1 force-pushed the taskrun-list-page branch from e82afd7 to d5876d9 Compare October 15, 2020 09:07

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2020

debsmita1 force-pushed the taskrun-list-page branch from d5876d9 to 771fbf8 Compare October 15, 2020 10:12

debsmita1 requested review from bgliwa01 and removed request for abhi-kn October 15, 2020 10:14

Update TaskRun List Page Columns & PipelineRun Tab

1867591

debsmita1 force-pushed the taskrun-list-page branch from 771fbf8 to 1867591 Compare October 15, 2020 14:37

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2020

bgliwa01 approved these changes Oct 15, 2020

View reviewed changes

rohitkrai03 approved these changes Oct 19, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 19, 2020

openshift-merge-robot merged commit 8373773 into openshift:master Oct 20, 2020

spadgett added this to the v4.7 milestone Oct 21, 2020

Update TaskRun List Page Columns & PipelineRun Tab #6840

Update TaskRun List Page Columns & PipelineRun Tab #6840

Conversation

debsmita1 commented Oct 5, 2020 • edited

debsmita1 commented Oct 5, 2020

debsmita1 commented Oct 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

debsmita1 commented Oct 6, 2020

abhinandan13jan Oct 6, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgliwa01 commented Oct 8, 2020

bgliwa01 commented Oct 8, 2020

debsmita1 commented Oct 8, 2020

andrewballantyne commented Oct 8, 2020

bgliwa01 commented Oct 8, 2020

bgliwa01 commented Oct 8, 2020

andrewballantyne commented Oct 8, 2020

jerolimov left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karthikjeeyar Oct 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgliwa01 left a comment

Choose a reason for hiding this comment

karthikjeeyar commented Oct 13, 2020

debsmita1 commented Oct 15, 2020

debsmita1 commented Oct 15, 2020

karthikjeeyar commented Oct 15, 2020

bgliwa01 left a comment

Choose a reason for hiding this comment

rohitkrai03 left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 19, 2020

debsmita1 commented Oct 19, 2020

debsmita1 commented Oct 19, 2020

openshift-bot commented Oct 19, 2020

openshift-bot commented Oct 19, 2020

openshift-bot commented Oct 19, 2020

openshift-bot commented Oct 19, 2020

debsmita1 commented Oct 5, 2020 •

edited

abhinandan13jan Oct 6, 2020 •

edited

jerolimov left a comment •

edited

karthikjeeyar Oct 12, 2020 •

edited