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

Feature/optimize search #120

Merged

Conversation

mikemorency
Copy link
Collaborator

SUMMARY

Implements a much faster search, using the propertyCollector class from pyvmomi. This was originally brought up in the discussion here: eddiehavila/community.vmware#1 (reply in thread)

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

pyvmomi base class and client

@mikemorency mikemorency force-pushed the feature/optimize-search branch from e3c7dd3 to 7551755 Compare February 18, 2025 00:35
Copy link

codecov bot commented Feb 18, 2025

Codecov Report

Attention: Patch coverage is 97.20000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 64.87%. Comparing base (e23704a) to head (4a1e893).
Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
plugins/module_utils/clients/_pyvmomi.py 73.68% 3 Missing and 2 partials ⚠️
plugins/module_utils/_module_pyvmomi_base.py 96.49% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #120      +/-   ##
==========================================
+ Coverage   61.65%   64.87%   +3.22%     
==========================================
  Files          60       60              
  Lines        3943     4151     +208     
  Branches      621      636      +15     
==========================================
+ Hits         2431     2693     +262     
+ Misses       1349     1296      -53     
+ Partials      163      162       -1     
Flag Coverage Δ
units 64.87% <97.20%> (+3.22%) ⬆️

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.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mikemorency mikemorency marked this pull request as ready for review February 19, 2025 18:51
@mikemorency mikemorency requested review from mariolenz and OrrBG and removed request for elsapassaro February 19, 2025 18:57
@mikemorency mikemorency mentioned this pull request Feb 25, 2025
@mariolenz
Copy link
Collaborator

@mikemorency I'd really like to have a closer look at this and how it works in community.vmware. But I'll have to refactor some things there in order to test this.

Please beare with me some more days 🙏

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

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

I'm still trying to wrap my head around those changes. It's a little bit complicated, since I've lost track about a lot of the changes in this collection.

Anyway, while looking into the question how I can re-create your changes in community.vmware in order to test them I ran into another question. I think discussing this would be a little bit off-topic here, so I've created #133 to discuss this.

@mikemorency
Copy link
Collaborator Author

@mariolenz FYI Id like to merge this in the next few days

@mariolenz
Copy link
Collaborator

I'd like to test this approach in community.vmware, just to make sure that it works there also. I didn't find the time to do this yet, though.

I still want to do this. But if you want to merge and I didn't find the time to test until then, don't wait for me! As far as I can see, the code LGTM. I just thought it might be helpful to see how this works in community.vmware.

If in doubt, just merge and don't wait. When I find the time to tests this in the other collection, I'll let you know the result. No matter if this PR has been merged by then or not.

@mikemorency mikemorency force-pushed the feature/optimize-search branch from 4c3aabf to 4a1e893 Compare March 19, 2025 15:42
@mikemorency mikemorency merged commit 2270790 into ansible-collections:main Mar 19, 2025
19 checks passed
@mikemorency mikemorency deleted the feature/optimize-search branch March 19, 2025 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants