feat(tkresults): update filters api call #1019

abhinandan13jan · 2024-11-05T13:15:50Z

Fixes

Please note that the new logs call takes a lot of network time.

https://issues.redhat.com/browse/KFLUXUI-40

Description

Type of change

Screen shots / Gifs for design review

How to test or reproduce?

Browser conformance:

Chrome
Firefox
Safari
Edge

codecov · 2024-11-07T12:45:36Z

Codecov Report

Attention: Patch coverage is 97.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.37%. Comparing base (44f9653) to head (6fcc21b).
Report is 18 commits behind head on main.

Files with missing lines	Patch %	Lines
...ntractView/useEnterpriseContractResultFromLogs.tsx	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1019      +/-   ##
==========================================
+ Coverage   85.01%   85.37%   +0.36%     
==========================================
  Files         582      582              
  Lines       14332    14356      +24     
  Branches     4010     4020      +10     
==========================================
+ Hits        12184    12257      +73     
+ Misses       2021     1972      -49     
  Partials      127      127

Files with missing lines	Coverage Δ
...riseContractView/SecurityEnterpriseContractTab.tsx	`85.00% <ø> (ø)`
...mponents/TaskRunDetailsView/TaskRunDetailsView.tsx	`96.15% <100.00%> (+0.50%)`	⬆️
src/hooks/useTektonResults.ts	`95.71% <100.00%> (-2.75%)`	⬇️
...onents/pipeline-run-logs/logs/TektonTaskRunLog.tsx	`100.00% <100.00%> (ø)`
...ed/components/pipeline-run-logs/logs/logs-utils.ts	`78.16% <100.00%> (+42.35%)`	⬆️
src/utils/tekton-results.ts	`98.54% <100.00%> (+0.01%)`	⬆️
...ntractView/useEnterpriseContractResultFromLogs.tsx	`93.06% <80.00%> (-0.69%)`	⬇️

... and 2 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

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

---- 🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

CryptoRodeo · 2024-11-07T21:32:46Z

src/hooks/__tests__/useTektonResults.spec.ts

 const getPipelineRunsMock = getPipelineRuns as jest.Mock;
 const getTaskRunsMock = getTaskRuns as jest.Mock;
-const getTaskRunLogMock = getTaskRunLog as jest.Mock;
+// const getTaskRunLogMock = getTaskRunLog as jest.Mock;


Was this meant to be left commented out?

ya, will remove this

CryptoRodeo · 2024-11-07T21:40:13Z

src/components/EnterpriseContractView/SecurityEnterpriseContractTab.tsx

-> = ({ pipelineRun }) => {
-  const [ecResult, ecResultLoaded] = useEnterpriseContractResults(pipelineRun);
+  React.PropsWithChildren<{
+    pipelineRunName: string;


Would it make sense to pass the full pipeline run object here rather than the metadata values?

I have no objections with either strategy, I was just wondering if there was a reason why we extract the values, then pass to the component instead of passing the full object and minimizing the props.

No, I dont think we should use the full pipeline as a prop as any change in any unrelated portion of the pipeline i.e pipeline.spec or pipeline.status will trigger a re-render of the component.

Good point! Thank you

sahil143

Can you access pipeline run uid and name from the the task run owner reference?

sahil143 · 2024-11-11T11:13:05Z

src/utils/tekton-results.ts

@@ -333,19 +340,29 @@ export const getTaskRuns = (
 const getLog = (workspace: string, taskRunPath: string) =>
  commonFetchText(`${getTRUrlPrefix(workspace)}/${taskRunPath.replace('/records/', '/logs/')}`);

-export const getTaskRunLog = (
+export const getTaskRunLogOld = (


Do we need this?

sahil143 · 2024-11-11T11:14:13Z

src/utils/tekton-results.ts

+) =>
+  commonFetchText(
+    `${getTRUrlPrefix(workspace)}/${namespace}/results/${pid}/logs/${taskRun?.metadata?.uid}`,
+  ).catch(() => getTaskRunLogOld(workspace, namespace, taskRun.metadata.name));


Why are you using getTaskRunLogOld?
If it fails to fetch we should just throw error and surface in UI.

openshift-ci · 2024-11-12T15:06:36Z

@abhinandan13jan: all tests passed!

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

abhinandan13jan · 2024-11-14T11:22:06Z

/retest

CryptoRodeo · 2024-11-18T15:01:17Z

/lgtm

sahil143

/approve

openshift-ci · 2024-11-19T11:30:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinandan13jan, sahil143

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~~ [sahil143]

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

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

openshift-ci bot requested review from karthikjeeyar and sahil143 November 5, 2024 13:16

feat(tkresults): update filters api call

a0f63f2

abhinandan13jan force-pushed the update-tekton-results branch from 9643210 to a0f63f2 Compare November 5, 2024 13:31

abhinandan13jan changed the title ~~[WIP]feat(tkresults): update filters api call~~ feat(tkresults): update filters api call Nov 6, 2024

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

abhinandan13jan force-pushed the update-tekton-results branch 2 times, most recently from 8b82bfc to af6140d Compare November 7, 2024 12:10

feat(tkn): fix tests

7f19945

abhinandan13jan force-pushed the update-tekton-results branch from af6140d to 7f19945 Compare November 7, 2024 15:58

CryptoRodeo reviewed Nov 7, 2024

View reviewed changes

feat(tkn): update tekton results

298ce7a

sahil143 suggested changes Nov 11, 2024

View reviewed changes

openshift-ci bot assigned sahil143 Nov 11, 2024

abhinandan13jan force-pushed the update-tekton-results branch 2 times, most recently from efa6d0b to 00838c9 Compare November 11, 2024 18:02

abhinandan13jan requested review from CryptoRodeo and sahil143 November 12, 2024 09:25

feat(tekton-results): update to use ownerReference id

6fcc21b

abhinandan13jan force-pushed the update-tekton-results branch from a3e651e to 6fcc21b Compare November 12, 2024 14:29

openshift-ci bot assigned CryptoRodeo Nov 18, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2024

sahil143 approved these changes Nov 19, 2024

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(tkresults): update filters api call #1019

feat(tkresults): update filters api call #1019

abhinandan13jan commented Nov 5, 2024 •

edited

Loading

codecov bot commented Nov 7, 2024 •

edited

Loading

CryptoRodeo Nov 7, 2024

abhinandan13jan Nov 8, 2024

CryptoRodeo Nov 7, 2024

abhinandan13jan Nov 8, 2024

CryptoRodeo Nov 8, 2024

sahil143 left a comment

sahil143 Nov 11, 2024

sahil143 Nov 11, 2024

abhinandan13jan Nov 11, 2024

openshift-ci bot commented Nov 12, 2024

abhinandan13jan commented Nov 14, 2024

CryptoRodeo commented Nov 18, 2024

sahil143 left a comment

openshift-ci bot commented Nov 19, 2024

feat(tkresults): update filters api call #1019

Are you sure you want to change the base?

feat(tkresults): update filters api call #1019

Conversation

abhinandan13jan commented Nov 5, 2024 • edited Loading

Fixes

Description

Type of change

Screen shots / Gifs for design review

How to test or reproduce?

Browser conformance:

codecov bot commented Nov 7, 2024 • edited Loading

Codecov Report

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

sahil143 left a 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

openshift-ci bot commented Nov 12, 2024

abhinandan13jan commented Nov 14, 2024

CryptoRodeo commented Nov 18, 2024

sahil143 left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Nov 19, 2024

abhinandan13jan commented Nov 5, 2024 •

edited

Loading

codecov bot commented Nov 7, 2024 •

edited

Loading