[ARVADOS] created: f9d2cd963c314188c0351253c00d6dc82f276ed7
git at public.curoverse.com
git at public.curoverse.com
Tue Aug 5 15:17:14 EDT 2014
at f9d2cd963c314188c0351253c00d6dc82f276ed7 (commit)
commit f9d2cd963c314188c0351253c00d6dc82f276ed7
Author: Brett Smith <brett at curoverse.com>
Date: Tue Aug 5 15:18:14 2014 -0400
3415: API exceptions from Python SDK include more error information.
The apiclient module doesn't give us a lot of opportunities to
customize error handling. Request objects can have response
callbacks, but they only get access to the response headers, not body,
which we need to pass along JSON errors. After that, apiclient.http
imports apiclient.errors.HttpError directly, and raises that directly
whenever there's a permanent error in an HTTP response.
arvados.api already makes a few monkeypatches to apiclient, and this
commit adds one more: it customizes HttpError's __new__ method to
return a new customized subclass instead. This is pretty evil,
because it will mess with any other instantiations of HttpError in
client programs. Its only mitigating grace is that the new subclass
is fully API-compatible with the original.
diff --git a/sdk/python/arvados/api.py b/sdk/python/arvados/api.py
index eb09664..8a71b90 100644
--- a/sdk/python/arvados/api.py
+++ b/sdk/python/arvados/api.py
@@ -7,6 +7,7 @@ import types
import apiclient
import apiclient.discovery
+import apiclient.errors
import config
import errors
import util
@@ -51,6 +52,15 @@ def _cast_objects_too(value, schema_type):
return _cast_orig(value, schema_type)
apiclient.discovery._cast = _cast_objects_too
+# Convert apiclient's HttpErrors into our own API error subclass for better
+# error reporting.
+# Reassigning apiclient.errors.HttpError is not sufficient because most of the
+# apiclient submodules import the class into their own namespace.
+def _new_http_error(cls, *args, **kwargs):
+ return super(apiclient.errors.HttpError, cls).__new__(
+ errors.ApiError, *args, **kwargs)
+apiclient.errors.HttpError.__new__ = staticmethod(_new_http_error)
+
def http_cache(data_type):
path = os.environ['HOME'] + '/.cache/arvados/' + data_type
try:
diff --git a/sdk/python/arvados/errors.py b/sdk/python/arvados/errors.py
index 85472d8..1d9c778 100644
--- a/sdk/python/arvados/errors.py
+++ b/sdk/python/arvados/errors.py
@@ -1,5 +1,16 @@
# errors.py - Arvados-specific exceptions.
+import apiclient.errors
+import json
+
+class ApiError(apiclient.errors.HttpError):
+ def _get_reason(self):
+ try:
+ return '; '.join(json.loads(self.content)['errors'])
+ except (KeyError, TypeError, ValueError):
+ return super(ApiError, self)._get_reason()
+
+
class ArgumentError(Exception):
pass
class SyntaxError(Exception):
diff --git a/sdk/python/tests/test_api.py b/sdk/python/tests/test_api.py
index cd7b1e9..4c48571 100644
--- a/sdk/python/tests/test_api.py
+++ b/sdk/python/tests/test_api.py
@@ -1,5 +1,6 @@
#!/usr/bin/env python
+import apiclient.errors
import arvados
import httplib2
import json
@@ -14,6 +15,19 @@ if not mimetypes.inited:
class ArvadosApiClientTest(unittest.TestCase):
@classmethod
+ def response_from_code(cls, code):
+ return httplib2.Response(
+ {'status': code,
+ 'reason': HTTP_RESPONSES.get(code, "Unknown Response"),
+ 'Content-Type': mimetypes.types_map['.json']})
+
+ @classmethod
+ def api_error_response(cls, code, *errors):
+ return (cls.response_from_code(code),
+ json.dumps({'errors': errors,
+ 'error_token': '1234567890+12345678'}))
+
+ @classmethod
def setUpClass(cls):
# The apiclient library has support for mocking requests for
# testing, but it doesn't extend to the discovery document
@@ -22,6 +36,9 @@ class ArvadosApiClientTest(unittest.TestCase):
cls.orig_api_host = arvados.config.get('ARVADOS_API_HOST')
arvados.config.settings()['ARVADOS_API_HOST'] = 'qr1hi.arvadosapi.com'
mock_responses = {
+ 'arvados.humans.delete': (cls.response_from_code(500), ""),
+ 'arvados.humans.get': cls.api_error_response(
+ 422, "Bad UUID format", "Bad output format"),
'arvados.humans.list': (None, json.dumps(
{'items_available': 0, 'items': []})),
}
@@ -42,6 +59,18 @@ class ArvadosApiClientTest(unittest.TestCase):
filters=[['uuid', 'is', None]]).execute()
self.assertEqual(answer['items_available'], len(answer['items']))
+ def test_exceptions_include_errors(self):
+ with self.assertRaises(apiclient.errors.HttpError) as err_ctx:
+ self.api.humans().get(uuid='xyz-xyz-abcdef').execute()
+ err_s = str(err_ctx.exception)
+ for msg in ["Bad UUID format", "Bad output format"]:
+ self.assertIn(msg, err_s)
+
+ def test_exceptions_without_errors_have_basic_info(self):
+ with self.assertRaises(apiclient.errors.HttpError) as err_ctx:
+ self.api.humans().delete(uuid='xyz-xyz-abcdef').execute()
+ self.assertIn("500", str(err_ctx.exception))
+
if __name__ == '__main__':
unittest.main()
commit 46f565cf2dd89a3ec6ad78b1237b3a4b0db6404b
Author: Brett Smith <brett at curoverse.com>
Date: Tue Aug 5 13:23:31 2014 -0400
3415: Python SDK can pass extra arguments to the client constructor.
This gives API authors more flexibility, and lets us write tests using
apiclient's built-in mock functionality. This is needed to test
#3415.
diff --git a/sdk/python/arvados/api.py b/sdk/python/arvados/api.py
index 699c319..eb09664 100644
--- a/sdk/python/arvados/api.py
+++ b/sdk/python/arvados/api.py
@@ -59,37 +59,57 @@ def http_cache(data_type):
path = None
return path
-def api(version=None, cache=True):
- global services
+def api(version=None, cache=True, **kwargs):
+ """Return an apiclient Resources object for an Arvados instance.
- if 'ARVADOS_DEBUG' in config.settings():
+ Arguments:
+ * version: A string naming the version of the Arvados API to use (for
+ example, 'v1').
+ * cache: If True (default), return an existing resources object, or use
+ a cached discovery document to build one.
+
+ Additional keyword arguments will be passed directly to
+ `apiclient.discovery.build`. If the `discoveryServiceUrl` or `http`
+ keyword arguments are missing, this function will set default values for
+ them, based on the current Arvados configuration settings."""
+ if config.get('ARVADOS_DEBUG'):
logging.basicConfig(level=logging.DEBUG)
if not cache or not services.get(version):
- apiVersion = version
if not version:
- apiVersion = 'v1'
+ version = 'v1'
logging.info("Using default API version. " +
"Call arvados.api('%s') instead." %
- apiVersion)
- if 'ARVADOS_API_HOST' not in config.settings():
- raise Exception("ARVADOS_API_HOST is not set. Aborting.")
- url = ('https://%s/discovery/v1/apis/{api}/{apiVersion}/rest' %
- config.get('ARVADOS_API_HOST'))
- credentials = CredentialsFromEnv()
+ version)
+
+ if 'discoveryServiceUrl' not in kwargs:
+ api_host = config.get('ARVADOS_API_HOST')
+ if not api_host:
+ raise ValueError(
+ "No discoveryServiceUrl or ARVADOS_API_HOST set.")
+ kwargs['discoveryServiceUrl'] = (
+ 'https://%s/discovery/v1/apis/{api}/{apiVersion}/rest' %
+ (api_host,))
- # Use system's CA certificates (if we find them) instead of httplib2's
- ca_certs = '/etc/ssl/certs/ca-certificates.crt'
- if not os.path.exists(ca_certs):
- ca_certs = None # use httplib2 default
+ if 'http' not in kwargs:
+ http_kwargs = {}
+ # Prefer system's CA certificates (if available) over httplib2's.
+ certs_path = '/etc/ssl/certs/ca-certificates.crt'
+ if os.path.exists(certs_path):
+ http_kwargs['ca_certs'] = certs_path
+ if cache:
+ http_kwargs['cache'] = http_cache('discovery')
+ if (config.get('ARVADOS_API_HOST_INSECURE', '').lower() in
+ ('yes', 'true', '1')):
+ http_kwargs['disable_ssl_certificate_validation'] = True
+ kwargs['http'] = httplib2.Http(**http_kwargs)
- http = httplib2.Http(ca_certs=ca_certs,
- cache=(http_cache('discovery') if cache else None))
- http = credentials.authorize(http)
- if re.match(r'(?i)^(true|1|yes)$',
- config.get('ARVADOS_API_HOST_INSECURE', 'no')):
- http.disable_ssl_certificate_validation=True
- services[version] = apiclient.discovery.build(
- 'arvados', apiVersion, http=http, discoveryServiceUrl=url)
- http.cache = None
+ kwargs['http'] = CredentialsFromEnv().authorize(kwargs['http'])
+ services[version] = apiclient.discovery.build('arvados', version,
+ **kwargs)
+ kwargs['http'].cache = None
return services[version]
+
+def uncache_api(version):
+ if version in services:
+ del services[version]
diff --git a/sdk/python/tests/test_api.py b/sdk/python/tests/test_api.py
new file mode 100644
index 0000000..cd7b1e9
--- /dev/null
+++ b/sdk/python/tests/test_api.py
@@ -0,0 +1,47 @@
+#!/usr/bin/env python
+
+import arvados
+import httplib2
+import json
+import mimetypes
+import unittest
+
+from apiclient.http import RequestMockBuilder
+from httplib import responses as HTTP_RESPONSES
+
+if not mimetypes.inited:
+ mimetypes.init()
+
+class ArvadosApiClientTest(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ # The apiclient library has support for mocking requests for
+ # testing, but it doesn't extend to the discovery document
+ # itself. Point it at a known stable discovery document for now.
+ # FIXME: Figure out a better way to stub this out.
+ cls.orig_api_host = arvados.config.get('ARVADOS_API_HOST')
+ arvados.config.settings()['ARVADOS_API_HOST'] = 'qr1hi.arvadosapi.com'
+ mock_responses = {
+ 'arvados.humans.list': (None, json.dumps(
+ {'items_available': 0, 'items': []})),
+ }
+ req_builder = RequestMockBuilder(mock_responses)
+ cls.api = arvados.api('v1', False, requestBuilder=req_builder)
+
+ @classmethod
+ def tearDownClass(cls):
+ if cls.orig_api_host is None:
+ del arvados.config.settings()['ARVADOS_API_HOST']
+ else:
+ arvados.config.settings()['ARVADOS_API_HOST'] = cls.orig_api_host
+ # Prevent other tests from using our mocked API client.
+ arvados.uncache_api('v1')
+
+ def test_basic_list(self):
+ answer = self.api.humans().list(
+ filters=[['uuid', 'is', None]]).execute()
+ self.assertEqual(answer['items_available'], len(answer['items']))
+
+
+if __name__ == '__main__':
+ unittest.main()
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list