StoragePool get_sdss doesn't work as expected

[stgaver]

There are several issues with the get_sdss method in storage_pool.py

Issue 1 - Getting the SDS associated with a storage pool and only requesting certain fields is failing. Root cause: In get_sdss when we do a .get on the Sds objects we are passing filter_fields and fields as positional arguments. This causes fields to be put into filter_fields. As a result we get the AttributeError since a list doesn’t have a .items. return Sds(self.token, self.configuration).get(filter_fields, fields) Fix: The fix here is to pass them as named args as in get(filter_fields=filter_fields, fields=fields) After this change, SDS IDs are returned, albeit incorrectly. See Issue 2.

Test script

from PyPowerFlex import PowerFlexClient
client = PowerFlexClient(gateway_address="10.46.138.164",
                         username='admin', password=‘*********’)
client.initialize()
for sds in client.storage_pool.get_sdss("1f5e866000000003", fields=['id']):
  print(sds['id'])

This fails with error:

Traceback (most recent call last):
  File "/home/sam.gaver/test/nutest-py3-tests/test.py", line 5, in <module>
    for sds in client.storage_pool.get_sdss("1f5e866000000003", fields=['id']):
  File "/home/sam.gaver/test/nutest-py3/env/lib/python3.9/site-packages/PyPowerFlex/objects/storage_pool.py", line 167, in get_sdss
    return Sds(self.token, self.configuration).get(filter_fields, fields)
  File "/home/sam.gaver/test/nutest-py3/env/lib/python3.9/site-packages/PyPowerFlex/base_client.py", line 311, in get
    response = utils.filter_response(response, filter_fields)
  File "/home/sam.gaver/test/nutest-py3/env/lib/python3.9/site-packages/PyPowerFlex/utils.py", line 66, in filter_response
    return list(filter(filter_func, response))
  File "/home/sam.gaver/test/nutest-py3/env/lib/python3.9/site-packages/PyPowerFlex/utils.py", line 52, in filter_func
    for filter_key, filter_value in filter_fields.items():
AttributeError: 'list' object has no attribute 'items'

Issue 2: ALL SDSs are returned for get_sdss for a storage_pool. get_sdss should only return the SDS associated with the defined storage pool. Root cause: If filter_fields isn’t passed to the get_sdss method filter_fields is never updated with the SDS IDs associated with the storage pool from the get_related call. Also the format of the filter_fields is incorrect. Fix: Add the following code

sds_id_list = [sds['sdsId'] for sds in sdss_ids]
        if filter_fields:
            filter_fields.update({'id': sds_id_list})
        else:
            filter_fields = {'id': sds_id_list}

With these two fixes the results are as expected from the test script:

0bb1eb5000000009
0bb1eb4f00000008
0bb1eb4e00000007

Issue 3: If passing filter_fields to get_sdss so that we only get one of the sds in this storage_pool we get zero sds. Root cause: The get_related call filters based on the field ‘sdsId’ so that is what you have to pass in filter_fields to get_sdss like: client.storage_pool.get_sdss("1f5e866000000003", filter_fields={'sdsId': ['0bb1eb5000000009']}) The filter_fields is then updated with {‘id’: ..} so filter_fields will have both sdsId and id in it. But both aren’t present so the filter fails and no SDS ids are returned. Fix: pop off sdsId from the filter_fields before trying to return the SDSs

In summary, all the changes:

sam.gaver@sam-gaver-1 [~/test/python-powerflex]: git diff
diff --git a/PyPowerFlex/objects/storage_pool.py b/PyPowerFlex/objects/storage_pool.py
index 23555e1..2992030 100644
--- a/PyPowerFlex/objects/storage_pool.py
+++ b/PyPowerFlex/objects/storage_pool.py
@@ -162,9 +162,14 @@ class StoragePool(base_client.EntityRequest):
                                     'SpSds',
                                     filter_fields,
                                     fields=('sdsId',))
+        sds_id_list = [sds['sdsId'] for sds in sdss_ids]
         if filter_fields:
-            filter_fields.update({'id': sdss_ids})
-        return Sds(self.token, self.configuration).get(filter_fields, fields)
+            filter_fields.update({'id': sds_id_list})
+            filter_fields.pop('sdsId', None)
+        else:
+            filter_fields = {'id': sds_id_list}
+        return Sds(self.token, self.configuration).get(
+            filter_fields=filter_fields, fields=fields)

     def get_volumes(self, storage_pool_id, filter_fields=None, fields=None):
         """Get related PowerFlex volumes for storage pool.

[anupamaloke]

@stgaver, thank you for identifying these issues and even providing a fix for these 👍. I would highly encourage you to submit a PR too.

[stgaver]

@anupamaloke , I was going to but I don't seem to have proper permissions to create a new branch.

[anupamaloke]

@stgaver, you should be able to fork the repo into your account, clone the forked repo, create a new branch and make the proposed code changes, commit and then submit a PR to this upstream repo.