From 4fd735476b69fd5e5d5c3162a752a8a5022d8d18 Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Wed, 11 Nov 2015 12:17:46 -0500 Subject: [PATCH] 7696: PySDK KeepClient uses all service types. Filter out gateway services from the list of usable services, rather than selecting only disk and proxy types. --- sdk/python/arvados/keep.py | 48 +++++++++++++++------------- sdk/python/tests/test_keep_client.py | 17 ++++++++++ 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py index b3ccc3eb63..b3d64a4198 100644 --- a/sdk/python/arvados/keep.py +++ b/sdk/python/arvados/keep.py @@ -652,12 +652,13 @@ class KeepClient(object): self._gateway_services = {} self._keep_services = [{ 'uuid': 'proxy', + 'service_type': 'proxy', '_service_root': proxy, }] self._writable_services = self._keep_services self.using_proxy = True self._static_services_list = True - self.max_replicas_per_service = 1 + self.max_replicas_per_service = None else: # It's important to avoid instantiating an API client # unless we actually need one, for testing's sake. @@ -687,6 +688,10 @@ class KeepClient(object): t = self.proxy_timeout if self.using_proxy else self.timeout return (t[0] * (1 << attempt_number), t[1]) + def _any_nondisk_services(self, service_list): + return any(ks.get('service_type', 'disk') != 'disk' + for ks in service_list) + def build_services_list(self, force_rebuild=False): if (self._static_services_list or (self._keep_services and not force_rebuild)): @@ -697,12 +702,16 @@ class KeepClient(object): except Exception: # API server predates Keep services. keep_services = self.api_client.keep_disks().list() - accessible = keep_services.execute().get('items') - if not accessible: + # Gateway services are only used when specified by UUID, + # so there's nothing to gain by filtering them by + # service_type. + self._gateway_services = {ks['uuid']: ks for ks in + keep_services.execute()['items']} + if not self._gateway_services: raise arvados.errors.NoKeepServersError() # Precompute the base URI for each service. - for r in accessible: + for r in self._gateway_services.itervalues(): host = r['service_host'] if not host.startswith('[') and host.find(':') >= 0: # IPv6 URIs must be formatted like http://[::1]:80/... @@ -712,27 +721,19 @@ class KeepClient(object): host, r['service_port']) - # Gateway services are only used when specified by UUID, - # so there's nothing to gain by filtering them by - # service_type. - self._gateway_services = {ks.get('uuid'): ks for ks in accessible} _logger.debug(str(self._gateway_services)) - self._keep_services = [ - ks for ks in accessible - if ks.get('service_type') in ['disk', 'proxy']] - self._writable_services = [ - ks for ks in accessible - if (ks.get('service_type') in ['disk', 'proxy']) and (True != ks.get('read_only'))] - _logger.debug(str(self._keep_services)) - - self.using_proxy = any(ks.get('service_type') == 'proxy' - for ks in self._keep_services) + ks for ks in self._gateway_services.itervalues() + if not ks.get('service_type', '').startswith('gateway:')] + self._writable_services = [ks for ks in self._keep_services + if not ks.get('read_only')] + # For disk type services, max_replicas_per_service is 1 - # It is unknown or unlimited for non-disk typed services. - for ks in accessible: - if ('disk' != ks.get('service_type')) and (not ks.get('read_only')): - self.max_replicas_per_service = None + # It is unknown (unlimited) for other service types. + if self._any_nondisk_services(self._writable_services): + self.max_replicas_per_service = None + else: + self.max_replicas_per_service = 1 def _service_weight(self, data_hash, service_uuid): """Compute the weight of a Keep service endpoint for a data @@ -769,7 +770,8 @@ class KeepClient(object): # in that order. use_services = self._keep_services if need_writable: - use_services = self._writable_services + use_services = self._writable_services + self.using_proxy = self._any_nondisk_services(use_services) sorted_roots.extend([ svc['_service_root'] for svc in sorted( use_services, diff --git a/sdk/python/tests/test_keep_client.py b/sdk/python/tests/test_keep_client.py index ab57ed530e..a5d9925a75 100644 --- a/sdk/python/tests/test_keep_client.py +++ b/sdk/python/tests/test_keep_client.py @@ -387,6 +387,23 @@ class KeepClientServiceTestCase(unittest.TestCase, tutil.ApiClientMock): self.assertEqual(True, ("no Keep services available" in str(exc_check.exception))) self.assertEqual(0, len(exc_check.exception.request_errors())) + def test_oddball_service_get(self): + body = 'oddball service get' + api_client = self.mock_keep_services(service_type='fancynewblobstore') + with tutil.mock_keep_responses(body, 200): + keep_client = arvados.KeepClient(api_client=api_client) + actual = keep_client.get(tutil.str_keep_locator(body)) + self.assertEqual(body, actual) + + def test_oddball_service_put(self): + body = 'oddball service put' + pdh = tutil.str_keep_locator(body) + api_client = self.mock_keep_services(service_type='fancynewblobstore') + with tutil.mock_keep_responses(pdh, 200): + keep_client = arvados.KeepClient(api_client=api_client) + actual = keep_client.put(body, copies=1) + self.assertEqual(pdh, actual) + @tutil.skip_sleep class KeepClientRendezvousTestCase(unittest.TestCase, tutil.ApiClientMock): -- 2.30.2