swevm / scaleio-py

ScaleIO API Bindings
Apache License 2.0
13 stars 14 forks source link

fix: let get_sdc_for_volume return empty list if no SDCs are mapped #4

Closed wallnerryan closed 9 years ago

wallnerryan commented 9 years ago

Fix get_sdc_for_volume so that it returns an empty list if there are no SDC's connected. This previous version would fail with TypeError: 'NoneType' object is not iterable when trying to do something like delete_volume if no SDCs were attached.

>>> from scaleiopy import scaleio
>>> sio = scaleio.ScaleIO("https://192.168.50.12/api","admin","Scaleio123",verify_ssl=False)
2015-05-04 11:48:26,518: DEBUG scaleio:__init__ | Logger initialized!
>>> sio.create_volume_by_pd_name('testvol001', 8192, sio.get_pd_by_name('pdomain'))
.
.
.
<Response [200]>
>>> sio.volumes
.
.
.
mappingToAllSdcsEnabled: False
                    name: testvol001
                 size_kb: 8388608
         storage_pool_id: b731648f00000000
               use_cache: True
             volume_type: ThinProvisioned
                vtree_id: f58a4d1000000001]
.
.
.
>>> sio.delete_volume(sio.get_volume_by_name('testvol001'))
.
.
.
<Response [200]>
>>>
swevm commented 9 years ago

I really appreciate you trying to squeeze bugs out. There's likely plenty of them.

I'm not sure returning a empty list Is the correct way to go. Have been thinking of using exceptions instead or returning none/empty values/lists but haven't looked into it much more due to limited time.

wallnerryan commented 9 years ago

no problem, and I can help with adding in exceptions as they are generally good practice, however, an empty list in this use case of deleting a volume is the correct behavior, as a volume with no SDC's should still be able to be deleted without causing and error when it has no SDCs mapped.