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

fix: performance errors when listing pods #10199

Closed
wants to merge 2 commits into from

Conversation

amandavialva01
Copy link
Contributor

@amandavialva01 amandavialva01 commented Nov 5, 2024

Ticket

CM-594

Description

fix performance errors when listing pods

Test Plan

CI passes (automated testing)
I'll also add a manual test plan for this

Checklist

  • Changes have been manually QA'd
  • New features have been approved by the corresponding PM
  • User-facing API changes have the "User-facing API Change" label
  • Release notes have been added as a separate file under docs/release-notes/
    See Release Note for details.
  • Licenses have been included for new code which was copied and/or modified from any external code

@amandavialva01 amandavialva01 requested a review from a team as a code owner November 5, 2024 22:53
@amandavialva01 amandavialva01 requested review from hamidzr and removed request for a team November 5, 2024 22:53
@cla-bot cla-bot bot added the cla-signed label Nov 5, 2024
Copy link

codecov bot commented Nov 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 43.31%. Comparing base (b189d2a) to head (119f283).

❗ There is a different number of reports uploaded between BASE (b189d2a) and HEAD (119f283). Click for more details.

HEAD has 7 uploads less than BASE
Flag BASE (b189d2a) HEAD (119f283)
harness 9 3
web 1 0
Additional details and impacted files
@@                 Coverage Diff                 @@
##           release-0.35.0   #10199       +/-   ##
===================================================
- Coverage           54.00%   43.31%   -10.70%     
===================================================
  Files                1259      174     -1085     
  Lines              155373    18072   -137301     
  Branches             3498        0     -3498     
===================================================
- Hits                83916     7828    -76088     
+ Misses              71311    10244    -61067     
+ Partials              146        0      -146     
Flag Coverage Δ
harness 43.25% <ø> (-29.43%) ⬇️
web ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1186 files with indirect coverage changes

@amandavialva01 amandavialva01 force-pushed the amanda/CM594 branch 3 times, most recently from f25211a to 0316375 Compare November 6, 2024 13:59
return nil, err
}
c.QPS = 100
c.Burst = 100
return rest.InClusterConfig()
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this return c, err?

Copy link
Contributor Author

@amandavialva01 amandavialva01 Nov 7, 2024

Choose a reason for hiding this comment

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

This was ruled out as a viable solution before, I was testing solutions for a working fallback on a diff branch and I think i accidentally pushed this change here, I'll get rid of this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but yea it woudlve had to be return c, err

@amandavialva01 amandavialva01 deleted the amanda/CM594 branch November 14, 2024 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants