search

slipstream / SlipStreamClient

SlipStream Python client
Apache License 2.0
1 stars 4 forks source link

Fix bad usage of _vm_get_id in in _get_failed_instances_on_iaas and add method #361

Closed 0xbase12 closed 6 years ago

0xbase12 commented 6 years ago

Connected to SixSq/tasklist#1369 Connected to slipstream/SlipStreamConnectors#173

konstan commented 6 years ago

This is still discussed in this PR https://github.com/slipstream/SlipStreamConnectors/pull/174. I see no point in expanding interface just to satisfy one call. Especially with the initial idea of list_instances() -- check its definition here https://github.com/slipstream/SlipStreamClient/blob/master/client/src/main/python/slipstream/cloudconnectors/BaseCloudConnector.py#L91 -- and to avoid exactly this https://github.com/SixSq/SlipStreamConnectors/pull/113/commits/c38d8b09ae3ddc1dabd0a52fc76590b78cf6131a

The correct solution (as commented in https://github.com/slipstream/SlipStreamConnectors/pull/174) is overloaded _vm_get_id. Which in turn should be pulled up as a common implementation which would satisfy all connectors

@staticmethod
def _vm_get_id(vm):
    if hasattr(vm, 'id'):
        return vm.id
    else:
        return vm['id']
konstan commented 6 years ago

... and just noticed this as well https://github.com/slipstream/SlipStreamConnectors/pull/177/commits/99bdcc9ea0bb6cc5ea611088f0702a10eaccba58

konstan commented 6 years ago

Following the comments above, the correct and agreed solution is proposed here #362. Due to lack of time at the moment to implement and especially test #362, accepting this PR after discussion with @0xbase12 and @schaubl.