-
Notifications
You must be signed in to change notification settings - Fork 9
Quick vsphere fix #112
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
base: beta
Are you sure you want to change the base?
Quick vsphere fix #112
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question, otherwise LGTM
rubrik_polaris/compute/vsphere.py
Outdated
num_unmatched_criteria -= 1 | ||
if match_all and num_unmatched_criteria == 0: | ||
object_ids.append(instance['id']) | ||
elif not match_all and num_criteria > num_unmatched_criteria >= 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the intent of num_criteria > num_unmatched_criteria >= 1
to return False
when we match all criteria? (i.e. if we have match_all = False
, num_criteria = 5
and matches them all num_unmatched_criteria = 0
) ?
My impression was that it would be enough to match a single criteria to append when match_all = False
, i.e. drop the >= 1
:
elif not match_all and num_criteria > num_unmatched_criteria:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just ripped all of that out, since we implemented filtering in the resolver now. Take another look. Way simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, indeed, nice!
""" | ||
try: | ||
query_name = "compute_vmware_vsphere" | ||
query_name = "compute_vmware_vsphere_list" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This query file is missing, forgot to commit?
No description provided.