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

Add minimum details parameter to Search View's listDomains #9616

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

gpordeus
Copy link
Collaborator

Description

This PR adds the details: min parameter to listDomains API call. Since they are used only for ids and names, more details are unnecessary and listDomains ends up taking a long time to execute when many domains exist.

In a test environment with 5000 domains, running time cmk listDomains yielded cmk listDomains 0,31s user 0,11s system 1% cpu 31,200 total, but time cmk listDomains details=min yielded cmk listDomains details=min 0,14s user 0,04s system 7% cpu 2,521 total.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Screenshots (if appropriate):

Before:
image

After:
image

How Has This Been Tested?

I set up an environment with 5000 domains and unlimited page size.

I then checked the Network tab on the browser before and after the changes (see Screenshots).

Searching itself worked normally.

Copy link

codecov bot commented Aug 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.57%. Comparing base (d7ca05e) to head (a8f1503).
Report is 24 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9616      +/-   ##
============================================
- Coverage     15.57%   15.57%   -0.01%     
- Complexity    12037    12048      +11     
============================================
  Files          5501     5505       +4     
  Lines        482219   482621     +402     
  Branches      62044    60641    -1403     
============================================
+ Hits          75125    75175      +50     
- Misses       398791   399140     +349     
- Partials       8303     8306       +3     
Flag Coverage Δ
uitests 4.16% <ø> (-0.01%) ⬇️
unittests 16.35% <ø> (-0.01%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DaanHoogland
Copy link
Contributor

@blueorangutan ui

@blueorangutan
Copy link

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

Copy link
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

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

Code LGTM. I guess this should be done for other forms too

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/9616 (QA-JID-437)

@DaanHoogland
Copy link
Contributor

Code LGTM. I guess this should be done for other forms too

most probably, but I think we can merge this (and create an issue for the rest or a discussion to do the inventarisation)

@rohityadavcloud rohityadavcloud added this to the 4.20.0.0 milestone Sep 4, 2024
@rohityadavcloud rohityadavcloud merged commit 628aba6 into apache:main Sep 4, 2024
25 checks passed
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Sep 6, 2024
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.

5 participants