Apply property filters

[skanthed]

This commit introduces support for filtering nodes based on properties in the node listing command and adds corresponding test cases to ensure the functionality works as expected.

Changes in utils.py:

• Added functions to parse property filters and apply them to nodes.
  • convert_value: Converts a value string to the appropriate type (int, float, or string).
  • parse_property_filter: Parses a property filter string into a key, operator, and value.
  • node_matches_property_filters: Checks if a node matches all provided property filters.
  • filter_nodes_by_properties: Filters a list of nodes based on the provided property filters. Prints a message if no nodes match the specified properties.

Changes in node.py lease.py and offer.py:

• Updated the ListNode command to include property filtering support.
  • Added --property argument to the parser for specifying property filters.
  • Applied property filters to the list of nodes retrieved from the client.
  • Printed a message if no nodes, lease or offer match the specified properties.

Created test_utils.py:

Written additional test cases to verify the functionality of the utility functions:

• test_convert_value: Tests value conversion for different data types.
• test_parse_property_filter: Tests parsing of property filter strings.
• test_node_matches_property_filters: Tests node matching against multiple property filters.
• test_filter_nodes_by_properties: Tests the filtering of nodes based on property filters.

Changes in test_node.py, test_lease.py, test_offer.py:

• Added test cases to verify the functionality of property filters in node listing.
  • test_node_list_with_property_filter: Verifies node listing with a property filter (cpus>=40).
  • test_node_list_long_with_property_filter: Verifies detailed node listing with a property filter (memory_mb>=131072).

[tzumainn]

Looks like a good start! Two broad comments I'd make here:

• it probably makes sense to break off this logic into the utils file; this way it can be applied to any result that returns a list of properties
• you may want a dictionary that maps property names to the types of values they have (i.e., local_gb is a number, vendor is a string). I think this will allow you to do comparision operations

[skanthed]

Sure I will remember to keep it in the utils file. All values returned within properties are in string format. However, converting some values to integers to apply conditional logic.

[tzumainn]

I think this is a good start! The = operator works well for me; however, the > and >= operators failed:

$ openstack esi node list --property memory_mb>262144
Invalid property filter format: memory_mb

[skanthed]

I think this is a good start! The = operator works well for me; however, the > and >= operators failed:

$ openstack esi node list --property memory_mb>262144
Invalid property filter format: memory_mb

@tzumainn The command openstack esi node list --long --property memory_mb>262144 fails due to how the shell interprets the > character. Without quotes, the shell may treat > as a redirection operator, causing the argument to split incorrectly. Use quotes around the --property value to ensure it is passed as a single argument:

openstack esi node list --long --property "memory_mb>262144" 

[tzumainn]

I think it's really close! Looks like the code works; I'd just suggest rearranging a few things.

[larsks]

I would like to see tests in this pull request. We should exercise all the valid operators as well as the behavior when there is an invalid operator.

I would also like to see the commit message updated with a description of what this change is for. Currently neither the pull request not the commit has any description.

[skanthed]

@larsks @tzumainn - I have described this PR above. Please have a look.

[skanthed]

@tzumainn I have just written two test cases for now please let me know if you think I should write test different cases for every valid operator and test invalid operators too. I will do that.
In the final commit, I will provide detailed description of this PR.

[tzumainn]

Two broad comments!

• We also want to add the filter to offer/lease list; I believe those already have node properties in the --long version
• The unit tests should be done a little differently. In test_node.py you really only need to check that the call to utils.filter_nodes_by_properties is made; you can even mock the return value. Then you need to create and add tests to tests/unit/common/test_utils.py that call the utility functions directly and test the various outcomes. You'll want to test the various operators, and also the various possible inputs.

[skanthed]

@larsks @tzumainn I Updated the operator function as you suggested and test cases too. Once everything looks good for node list will update that in lease and offer list.

[tzumainn]

Left a few comments, including ones about missing test cases. If a test case gets long, just split it into multiples.

[skanthed]

@tzumainn I have addressed all your comments.

[tzumainn]

This looks pretty good - only a few minor changes. Can you also add a description to the commit message, and add the property filtering to the lease/offer lists?

[larsks]

I think this looks good! I've left a minor comment, but you can ignore it or not as you like.

[tzumainn]

Works great from my testing! Let me know if you wanted to make the change Lars suggested.

[skanthed]

@tzumainn @larsks Updated with your suggestion.

[tzumainn]

Looks good to me - thanks!