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

Filter VMs based on concerns label #778

Merged
merged 1 commit into from
Nov 30, 2023
Merged

Conversation

rszwajko
Copy link
Contributor

@rszwajko rszwajko commented Nov 8, 2023

Depends on #779

Screenshot from 2023-11-10 15-48-36

Screenshot from 2023-11-08 20-06-12

@ahadas
Copy link
Member

ahadas commented Nov 9, 2023

from UI perspective, I think the screenshots look really nice and useful
there's the part of how to represent the data that we talked about yesterday offline that I need more information on

@ahadas ahadas mentioned this pull request Nov 9, 2023
4 tasks
Copy link
Member

@yaacov yaacov left a comment

Choose a reason for hiding this comment

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

The VM list is expected to contain large number of VMs, N >> 1000
Dynamically fetching labels in this case is not optimal.

For the case of a list that is expected to hold large number of items for most expected cases, I prefer a hard coded list of options.

discussion is in: #764

IMHO we will need to create a hard coded list of all potential warnings and errors labels, or filter using a new field that will have more options then category while still being predefined and well known so we can have a hard coded list of this new field strings

cc: @ahadas @rszwajko

@rszwajko
Copy link
Contributor Author

@yaacov

The VM list is expected to contain large number of VMs, N >> 1000
Dynamically fetching labels in this case is not optimal.

To be precise we do not fetch the labels but only extract them from the VMs that we already have in-memory on the client side. We iterate over this array, filter and sort it i.e. see


IMHO one more iteration to extract the labels won't change much.

@rszwajko
Copy link
Contributor Author

Force pushed to rebase on #779

Create the list of available labels dynamically by traversing the list
of already fetched VMs.

Signed-off-by: Radoslaw Szwajkowski <[email protected]>
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
8.9% 8.9% Duplication

@rszwajko rszwajko marked this pull request as ready for review November 30, 2023 09:33
@rszwajko rszwajko requested a review from yaacov November 30, 2023 09:33
@rszwajko
Copy link
Contributor Author

Forced push to rebase + improved error handling
@yaacov as agreed a constant identifier assigned to each concern would be beneficial. However until it gets implemented I would propose to use the dynamic discovery as-it-is. If we hit any perf issues then we can re-evaluate.

@yaacov yaacov merged commit 0bea166 into kubev2v:main Nov 30, 2023
8 checks passed
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.

3 participants