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

OCPBUGS-23749: Improve PipelineRun list page responsiveness #13469

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
"Started": "Started",
"Signed": "Signed",
"Archived in Tekton results": "Archived in Tekton results",
"ago": "ago",
"Unknown failure condition": "Unknown failure condition",
"Failure on task {{taskName}} - check logs for details.": "Failure on task {{taskName}} - check logs for details.",
"Error downloading logs.": "Error downloading logs.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@
&__results-indicator {
display: inline-block;
margin-left: var(--pf-global--spacer--xs);
> svg{
> svg {
color: var(--pf-global--Color--400);
}
}
&__started-timestamp {
display: flex;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Tooltip } from '@patternfly/react-core';
import { ArchiveIcon } from '@patternfly/react-icons';
import { useTranslation } from 'react-i18next';
import { TableData, RowFunctionArgs } from '@console/internal/components/factory';
import { Timestamp, ResourceLink } from '@console/internal/components/utils';
import { Timestamp, ResourceLink, truncateMiddle } from '@console/internal/components/utils';
import { referenceForModel } from '@console/internal/module/k8s';
import {
DELETED_RESOURCE_IN_K8S_ANNOTATION,
Expand Down Expand Up @@ -31,15 +31,17 @@ const pipelinerunReference = referenceForModel(PipelineRunModel);
type PLRStatusProps = {
obj: PipelineRunKind;
taskRuns: TaskRunKind[];
iconOnly?: boolean;
};

const PLRStatus: React.FC<PLRStatusProps> = ({ obj, taskRuns }) => {
const PLRStatus: React.FC<PLRStatusProps> = ({ obj, taskRuns, iconOnly = false }) => {
return (
<PipelineRunStatus
status={pipelineRunFilterReducer(obj)}
title={pipelineRunTitleFilterReducer(obj)}
pipelineRun={obj}
taskRuns={taskRuns}
iconOnly={iconOnly}
/>
);
};
Expand All @@ -54,7 +56,7 @@ const PipelineRunRow: React.FC<RowFunctionArgs<PipelineRunKind>> = ({ obj, custo
<TableData className={tableColumnClasses.name}>
<ResourceLink
kind={pipelinerunReference}
name={obj.metadata.name}
name={truncateMiddle(obj.metadata.name, { length: 12 })}
Copy link
Member

@jerolimov jerolimov Jan 8, 2024

Choose a reason for hiding this comment

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

The resource link name is also used as link! So the link tries to open:

http://localhost:13469/k8s/ns/christoph/tekton.dev~v1~PipelineRun/nodein…uie93

Which results for sure in a 404 Not found error when the user clicks on this link. 😨

image

namespace={obj.metadata.namespace}
data-test-id={obj.metadata.name}
nameSuffix={
Expand Down Expand Up @@ -85,15 +87,20 @@ const PipelineRunRow: React.FC<RowFunctionArgs<PipelineRunKind>> = ({ obj, custo
<PipelineRunVulnerabilities pipelineRun={obj} condensed />
</TableData>
<TableData className={tableColumnClasses.status}>
<PLRStatus obj={obj} taskRuns={PLRTaskRuns} />
<PLRStatus obj={obj} taskRuns={PLRTaskRuns} iconOnly />
Copy link
Member

Choose a reason for hiding this comment

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

On wide screens/windows I would expect still the label:

Can we hide the labels maybe just on sm-md and show it on lg+?

image

</TableData>
<TableData className={tableColumnClasses.taskStatus}>
<LinkedPipelineRunTaskStatus pipelineRun={obj} taskRuns={PLRTaskRuns} />
</TableData>
<TableData className={tableColumnClasses.started}>
<Timestamp timestamp={obj.status && obj.status.startTime} />
<div className="opp-pipeline-run-list__started-timestamp">
<Timestamp timestamp={obj.status && obj.status.startTime} omitSuffix />{' '}
<div>&nbsp; {t('pipelines-plugin~ago')}</div>
Copy link
Member

Choose a reason for hiding this comment

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

This   and the space shows two spaces here:

image

</div>
</TableData>
<TableData className={tableColumnClasses.duration}>
{pipelineRunDuration(obj, false)}
</TableData>
<TableData className={tableColumnClasses.duration}>{pipelineRunDuration(obj)}</TableData>
<TableData className={tableColumnClasses.actions}>
<ResourceKebabWithUserLabel
actions={getPipelineRunKebabActions(operatorVersion, taskRuns)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ export const tableColumnClasses = {
name: '',
namespace: '',
vulnerabilities: 'pf-m-hidden pf-m-visible-on-md',
status: 'pf-m-hidden pf-m-visible-on-sm',
status: 'pf-m-hidden pf-m-visible-on-sm pf-v5-u-w-10-on-lg',
taskStatus: 'pf-m-hidden pf-m-visible-on-lg',
started: 'pf-m-hidden pf-m-visible-on-lg',
duration: 'pf-m-hidden pf-m-visible-on-xl',
started: 'pf-m-hidden pf-m-visible-on-lg pf-v5-u-w-15-on-lg',
duration: 'pf-m-hidden pf-m-visible-on-xl pf-v5-u-w-10-on-lg',
actions: Kebab.columnClass,
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ type PipelineResourceStatusProps = {
status: string;
children?: React.ReactNode;
title?: string;
iconOnly?: boolean;
};
const PipelineResourceStatus: React.FC<PipelineResourceStatusProps> = ({
status,
children,
title,
iconOnly,
}) => (
<Status status={status} title={title}>
<Status status={status} title={title} iconOnly={iconOnly}>
{status === 'Failed' && React.Children.toArray(children).length > 0 && children}
</Status>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,19 @@ type PipelineRunStatusProps = {
pipelineRun: PipelineRunKind;
title?: string;
taskRuns: TaskRunKind[];
iconOnly?: boolean;
};
const PipelineRunStatus: React.FC<PipelineRunStatusProps> = ({
status,
pipelineRun,
title,
taskRuns,
iconOnly,
}) => {
const { t } = useTranslation();
return pipelineRun ? (
taskRuns.length > 0 ? (
<PipelineResourceStatus status={status} title={title}>
<PipelineResourceStatus status={status} title={title} iconOnly={iconOnly}>
<StatusPopoverContent
logDetails={getPLRLogSnippet(pipelineRun, taskRuns)}
namespace={pipelineRun.metadata.namespace}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,15 +354,18 @@ export const calculateDuration = (startTime: string, endTime?: string, long?: bo
return getDuration(durationInSeconds, long);
};

export const pipelineRunDuration = (run: PipelineRunKind | TaskRunKind): string => {
export const pipelineRunDuration = (
run: PipelineRunKind | TaskRunKind,
long: boolean = true,
): string => {
const startTime = run?.status?.startTime ?? null;
const completionTime = run?.status?.completionTime ?? null;

// Duration cannot be computed if start time is missing or a completed/failed pipeline/task has no end time
if (!startTime || (!completionTime && pipelineRunStatus(run) !== 'Running')) {
return '-';
}
return calculateDuration(startTime, completionTime, true);
return calculateDuration(startTime, completionTime, long);
};

export const updateServiceAccount = (
Expand Down