[arvados] created: 2.1.0-3167-g9c872c735
git repository hosting
git at public.arvados.org
Thu Dec 8 15:53:01 UTC 2022
at 9c872c73562e66699eadf19863549de73ec7482d (commit)
commit 9c872c73562e66699eadf19863549de73ec7482d
Author: Brett Smith <brett.smith at curii.com>
Date: Thu Dec 8 10:51:58 2022 -0500
19686: Specify ThreadSafeApiCache API version throughout
This avoids logging a warning now that ThreadSafeApiCache accepts a
version argument just like other API client constructors.
Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>
diff --git a/sdk/cwl/arvados_cwl/__init__.py b/sdk/cwl/arvados_cwl/__init__.py
index 550ecba1c..1b0e1ea8e 100644
--- a/sdk/cwl/arvados_cwl/__init__.py
+++ b/sdk/cwl/arvados_cwl/__init__.py
@@ -328,7 +328,9 @@ def main(args=sys.argv[1:],
if api_client is None:
api_client = arvados.safeapi.ThreadSafeApiCache(
api_params={"model": OrderedJsonModel(), "timeout": arvargs.http_timeout},
- keep_params={"num_retries": 4})
+ keep_params={"num_retries": 4},
+ version='v1',
+ )
keep_client = api_client.keep
# Make an API object now so errors are reported early.
api_client.users().current().execute()
diff --git a/sdk/python/arvados/collection.py b/sdk/python/arvados/collection.py
index e1138910a..ebca15c54 100644
--- a/sdk/python/arvados/collection.py
+++ b/sdk/python/arvados/collection.py
@@ -1411,7 +1411,7 @@ class Collection(RichCollectionBase):
@synchronized
def _my_api(self):
if self._api_client is None:
- self._api_client = ThreadSafeApiCache(self._config)
+ self._api_client = ThreadSafeApiCache(self._config, version='v1')
if self._keep_client is None:
self._keep_client = self._api_client.keep
return self._api_client
diff --git a/services/fuse/arvados_fuse/command.py b/services/fuse/arvados_fuse/command.py
index 994c99882..8cb0a0601 100644
--- a/services/fuse/arvados_fuse/command.py
+++ b/services/fuse/arvados_fuse/command.py
@@ -227,7 +227,9 @@ class Mount(object):
disk_cache=self.args.disk_cache,
disk_cache_dir=self.args.disk_cache_dir),
'num_retries': self.args.retries,
- })
+ },
+ version='v1',
+ )
except KeyError as e:
self.logger.error("Missing environment: %s", e)
exit(1)
diff --git a/services/fuse/tests/mount_test_base.py b/services/fuse/tests/mount_test_base.py
index e82660408..c316010f6 100644
--- a/services/fuse/tests/mount_test_base.py
+++ b/services/fuse/tests/mount_test_base.py
@@ -54,7 +54,11 @@ class MountTestBase(unittest.TestCase):
run_test_server.run()
run_test_server.authorize_with("admin")
- self.api = api if api else arvados.safeapi.ThreadSafeApiCache(arvados.config.settings(), keep_params={"block_cache": make_block_cache(self.disk_cache)})
+ self.api = api if api else arvados.safeapi.ThreadSafeApiCache(
+ arvados.config.settings(),
+ keep_params={"block_cache": make_block_cache(self.disk_cache)},
+ version='v1',
+ )
self.llfuse_thread = None
# This is a copy of Mount's method. TODO: Refactor MountTestBase
diff --git a/services/fuse/tests/test_mount.py b/services/fuse/tests/test_mount.py
index df3d42634..a155acd14 100644
--- a/services/fuse/tests/test_mount.py
+++ b/services/fuse/tests/test_mount.py
@@ -1225,7 +1225,10 @@ class SlashSubstitutionTest(IntegrationTest):
def setUp(self):
super(SlashSubstitutionTest, self).setUp()
- self.api = arvados.safeapi.ThreadSafeApiCache(arvados.config.settings())
+ self.api = arvados.safeapi.ThreadSafeApiCache(
+ arvados.config.settings(),
+ version='v1',
+ )
self.api.config = lambda: {"Collections": {"ForwardSlashNameSubstitution": "[SLASH]"}}
self.testcoll = self.api.collections().create(body={"name": "foo/bar/baz"}).execute()
self.testcolleasy = self.api.collections().create(body={"name": "foo-bar-baz"}).execute()
@@ -1284,7 +1287,10 @@ class StorageClassesTest(IntegrationTest):
def setUp(self):
super(StorageClassesTest, self).setUp()
- self.api = arvados.safeapi.ThreadSafeApiCache(arvados.config.settings())
+ self.api = arvados.safeapi.ThreadSafeApiCache(
+ arvados.config.settings(),
+ version='v1',
+ )
@IntegrationTest.mount(argv=mnt_args)
def test_collection_default_storage_classes(self):
@@ -1321,7 +1327,7 @@ class ReadonlyCollectionTest(MountTestBase):
def runTest(self):
settings = arvados.config.settings().copy()
settings["ARVADOS_API_TOKEN"] = run_test_server.fixture("api_client_authorizations")["project_viewer"]["api_token"]
- self.api = arvados.safeapi.ThreadSafeApiCache(settings)
+ self.api = arvados.safeapi.ThreadSafeApiCache(settings, version='v1')
self.make_mount(fuse.CollectionDirectory, collection_record=self.testcollection, enable_write=False)
self.pool.apply(_readonlyCollectionTestHelper, (self.mounttmp,))
commit ee357f36efbd12c206972168222c8c08d8d0f88b
Author: Brett Smith <brett.smith at curii.com>
Date: Wed Dec 7 15:21:43 2022 -0500
19686: Fix test scaffolding after API changes
Without clearing arv-put's API client this way, multiple calls to main()
reuse the same, pre-patch Keep object from ThreadSafeApiCache, so calls
don't get recorded as intended.
Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>
diff --git a/sdk/python/tests/test_arv_put.py b/sdk/python/tests/test_arv_put.py
index 0e531dee3..afdf2238a 100644
--- a/sdk/python/tests/test_arv_put.py
+++ b/sdk/python/tests/test_arv_put.py
@@ -813,6 +813,7 @@ class ArvadosPutTest(run_test_server.TestCaseWithServers,
def test_put_block_replication(self):
self.call_main_on_test_file()
+ arv_put.api_client = None
with mock.patch('arvados.collection.KeepClient.local_store_put') as put_mock:
put_mock.return_value = 'acbd18db4cc2f85cedef654fccc4a4d8+3'
self.call_main_on_test_file(['--replication', '1'])
commit 9e1f21c9944e8d60212faddc4ddba087ec4a5cd7
Author: Brett Smith <brett.smith at curii.com>
Date: Tue Dec 6 23:16:28 2022 -0500
19686: Add tests for new Arvados client API
This commit ensures all public-facing functions have basic tests for
good inputs, as well as known-bad argument combinations (if any). This
includes testing functions like api_from_config that were previously
tested implicitly by other tests, but did not have their own dedicated
tests.
Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>
diff --git a/sdk/python/tests/test_api.py b/sdk/python/tests/test_api.py
index 03af8ce5e..20c4f346a 100644
--- a/sdk/python/tests/test_api.py
+++ b/sdk/python/tests/test_api.py
@@ -15,13 +15,22 @@ import os
import socket
import string
import unittest
+import urllib.parse as urlparse
import mock
from . import run_test_server
from apiclient import errors as apiclient_errors
from apiclient import http as apiclient_http
-from arvados.api import OrderedJsonModel, RETRY_DELAY_INITIAL, RETRY_DELAY_BACKOFF, RETRY_COUNT
+from arvados.api import (
+ api_client,
+ normalize_api_kwargs,
+ api_kwargs_from_config,
+ OrderedJsonModel,
+ RETRY_DELAY_INITIAL,
+ RETRY_DELAY_BACKOFF,
+ RETRY_COUNT,
+)
from .arvados_testutil import fake_httplib2_response, queue_with
if not mimetypes.inited:
@@ -36,6 +45,23 @@ class ArvadosApiTest(run_test_server.TestCaseWithServers):
json.dumps({'errors': errors,
'error_token': '1234567890+12345678'}).encode())
+ def _config_from_environ(self):
+ return {
+ key: value
+ for key, value in os.environ.items()
+ if key.startswith('ARVADOS_API_')
+ }
+
+ def _discoveryServiceUrl(
+ self,
+ host=None,
+ path='/discovery/v1/apis/{api}/{apiVersion}/rest',
+ scheme='https',
+ ):
+ if host is None:
+ host = os.environ['ARVADOS_API_HOST']
+ return urlparse.urlunsplit((scheme, host, path, None, None))
+
def test_new_api_objects_with_cache(self):
clients = [arvados.api('v1', cache=True) for index in [0, 1]]
self.assertIsNot(*clients)
@@ -160,6 +186,160 @@ class ArvadosApiTest(run_test_server.TestCaseWithServers):
self.assertTrue(hasattr(api_client, 'keep'),
f"client missing keep attribute")
+ def test_api_host_constructor(self):
+ cache = True
+ insecure = True
+ client = arvados.api(
+ 'v1',
+ cache,
+ os.environ['ARVADOS_API_HOST'],
+ os.environ['ARVADOS_API_TOKEN'],
+ insecure,
+ )
+ self.assertEqual(client.api_token, os.environ['ARVADOS_API_TOKEN'],
+ "client constructed with incorrect token")
+
+ def test_api_url_constructor(self):
+ client = arvados.api(
+ 'v1',
+ discoveryServiceUrl=self._discoveryServiceUrl(),
+ token=os.environ['ARVADOS_API_TOKEN'],
+ insecure=True,
+ )
+ self.assertEqual(client.api_token, os.environ['ARVADOS_API_TOKEN'],
+ "client constructed with incorrect token")
+
+ def test_api_bad_args(self):
+ all_kwargs = {
+ 'host': os.environ['ARVADOS_API_HOST'],
+ 'token': os.environ['ARVADOS_API_TOKEN'],
+ 'discoveryServiceUrl': self._discoveryServiceUrl(),
+ }
+ for use_keys in [
+ # Passing only a single key is missing required info
+ *([key] for key in all_kwargs.keys()),
+ # Passing all keys is a conflict
+ list(all_kwargs.keys()),
+ ]:
+ kwargs = {key: all_kwargs[key] for key in use_keys}
+ kwargs_list = ', '.join(use_keys)
+ with self.subTest(f"calling arvados.api with {kwargs_list} fails"), \
+ self.assertRaises(ValueError):
+ arvados.api('v1', insecure=True, **kwargs)
+
+ def test_api_bad_url(self):
+ for bad_kwargs in [
+ {'discoveryServiceUrl': self._discoveryServiceUrl() + '/BadTestURL'},
+ {'version': 'BadTestVersion', 'host': os.environ['ARVADOS_API_HOST']},
+ ]:
+ bad_key = next(iter(bad_kwargs))
+ with self.subTest(f"api fails with bad {bad_key}"), \
+ self.assertRaises(apiclient_errors.UnknownApiNameOrVersion):
+ arvados.api(**bad_kwargs, token='test_api_bad_url', insecure=True)
+
+ def test_normalize_api_good_args(self):
+ for version, discoveryServiceUrl, host in [
+ ('Test1', None, os.environ['ARVADOS_API_HOST']),
+ (None, self._discoveryServiceUrl(), None)
+ ]:
+ argname = 'discoveryServiceUrl' if host is None else 'host'
+ with self.subTest(f"normalize_api_kwargs with {argname}"):
+ actual = normalize_api_kwargs(
+ version,
+ discoveryServiceUrl,
+ host,
+ os.environ['ARVADOS_API_TOKEN'],
+ insecure=True,
+ )
+ self.assertEqual(actual['discoveryServiceUrl'], self._discoveryServiceUrl())
+ self.assertEqual(actual['token'], os.environ['ARVADOS_API_TOKEN'])
+ self.assertEqual(actual['version'], version or 'v1')
+ self.assertTrue(actual['insecure'])
+ self.assertNotIn('host', actual)
+
+ def test_normalize_api_bad_args(self):
+ all_args = (
+ self._discoveryServiceUrl(),
+ os.environ['ARVADOS_API_HOST'],
+ os.environ['ARVADOS_API_TOKEN'],
+ )
+ for arg_index, arg_value in enumerate(all_args):
+ args = [None] * len(all_args)
+ args[arg_index] = arg_value
+ with self.subTest(f"normalize_api_kwargs with only arg #{arg_index + 1}"), \
+ self.assertRaises(ValueError):
+ normalize_api_kwargs('v1', *args)
+ with self.subTest("normalize_api_kwargs with discoveryServiceUrl and host"), \
+ self.assertRaises(ValueError):
+ normalize_api_kwargs('v1', *all_args)
+
+ def test_api_from_config_default(self):
+ client = arvados.api_from_config('v1')
+ self.assertEqual(client.api_token, os.environ['ARVADOS_API_TOKEN'],
+ "client constructed with incorrect token")
+
+ def test_api_from_config_explicit(self):
+ config = self._config_from_environ()
+ client = arvados.api_from_config('v1', config)
+ self.assertEqual(client.api_token, os.environ['ARVADOS_API_TOKEN'],
+ "client constructed with incorrect token")
+
+ def test_api_from_bad_config(self):
+ base_config = self._config_from_environ()
+ for del_key in ['ARVADOS_API_HOST', 'ARVADOS_API_TOKEN']:
+ with self.subTest(f"api_from_config without {del_key} fails"), \
+ self.assertRaises(ValueError):
+ config = dict(base_config)
+ del config[del_key]
+ arvados.api_from_config('v1', config)
+
+ def test_api_kwargs_from_good_config(self):
+ for config in [None, self._config_from_environ()]:
+ conf_type = 'default' if config is None else 'passed'
+ with self.subTest(f"api_kwargs_from_config with {conf_type} config"):
+ version = 'Test1' if config else None
+ actual = api_kwargs_from_config(version, config)
+ self.assertEqual(actual['discoveryServiceUrl'], self._discoveryServiceUrl())
+ self.assertEqual(actual['token'], os.environ['ARVADOS_API_TOKEN'])
+ self.assertEqual(actual['version'], version or 'v1')
+ self.assertTrue(actual['insecure'])
+ self.assertNotIn('host', actual)
+
+ def test_api_kwargs_from_bad_config(self):
+ base_config = self._config_from_environ()
+ for del_key in ['ARVADOS_API_HOST', 'ARVADOS_API_TOKEN']:
+ with self.subTest(f"api_kwargs_from_config without {del_key} fails"), \
+ self.assertRaises(ValueError):
+ config = dict(base_config)
+ del config[del_key]
+ api_kwargs_from_config('v1', config)
+
+ def test_api_client_constructor(self):
+ client = api_client(
+ 'v1',
+ self._discoveryServiceUrl(),
+ os.environ['ARVADOS_API_TOKEN'],
+ insecure=True,
+ )
+ self.assertEqual(client.api_token, os.environ['ARVADOS_API_TOKEN'],
+ "client constructed with incorrect token")
+ self.assertFalse(
+ hasattr(client, 'localapi'),
+ "client has localapi method when it should not be thread-safe",
+ )
+
+ def test_api_client_bad_url(self):
+ all_args = ('v1', self._discoveryServiceUrl(), 'test_api_client_bad_url')
+ for arg_index, arg_value in [
+ (0, 'BadTestVersion'),
+ (1, all_args[1] + '/BadTestURL'),
+ ]:
+ with self.subTest(f"api_client fails with {arg_index}={arg_value!r}"), \
+ self.assertRaises(apiclient_errors.UnknownApiNameOrVersion):
+ args = list(all_args)
+ args[arg_index] = arg_value
+ api_client(*args, insecure=True)
+
class RetryREST(unittest.TestCase):
def setUp(self):
diff --git a/sdk/python/tests/test_safeapi.py b/sdk/python/tests/test_safeapi.py
new file mode 100644
index 000000000..855757a3e
--- /dev/null
+++ b/sdk/python/tests/test_safeapi.py
@@ -0,0 +1,63 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+import os
+import unittest
+
+import googleapiclient
+
+from arvados import safeapi
+
+from . import run_test_server
+
+class SafeApiTest(run_test_server.TestCaseWithServers):
+ MAIN_SERVER = {}
+
+ def test_constructor(self):
+ env_mapping = {
+ key: value
+ for key, value in os.environ.items()
+ if key.startswith('ARVADOS_API_')
+ }
+ extra_params = {
+ 'timeout': 299,
+ }
+ base_params = {
+ key[12:].lower(): value
+ for key, value in env_mapping.items()
+ }
+ try:
+ base_params['insecure'] = base_params.pop('host_insecure')
+ except KeyError:
+ pass
+ keep_params = {}
+ for config, params, subtest in [
+ (None, {}, "default arguments"),
+ (None, extra_params, "extra params"),
+ (env_mapping, {}, "explicit config"),
+ (env_mapping, extra_params, "explicit config and params"),
+ ({}, base_params, "params only"),
+ ]:
+ with self.subTest(f"test constructor with {subtest}"):
+ expected_timeout = params.get('timeout', 300)
+ expected_params = dict(params)
+ expected_keep_params = dict(keep_params)
+ client = safeapi.ThreadSafeApiCache(config, keep_params, params, 'v1')
+ self.assertTrue(hasattr(client, 'localapi'), "client missing localapi method")
+ self.assertEqual(client.api_token, os.environ['ARVADOS_API_TOKEN'])
+ self.assertEqual(client._http.timeout, expected_timeout)
+ self.assertEqual(params, expected_params,
+ "api_params was modified in-place")
+ self.assertEqual(keep_params, expected_keep_params,
+ "keep_params was modified in-place")
+
+ def test_constructor_no_args(self):
+ client = safeapi.ThreadSafeApiCache()
+ self.assertTrue(hasattr(client, 'localapi'), "client missing localapi method")
+ self.assertEqual(client.api_token, os.environ['ARVADOS_API_TOKEN'])
+ self.assertTrue(client.insecure)
+
+ def test_constructor_bad_version(self):
+ with self.assertRaises(googleapiclient.errors.UnknownApiNameOrVersion):
+ safeapi.ThreadSafeApiCache(version='BadTestVersion')
commit 2668643b7570db96651466250e7a496184f6ef0a
Author: Brett Smith <brett.smith at curii.com>
Date: Tue Nov 29 15:01:43 2022 -0500
19686: api constructor returns ThreadSafeApiCache
This is an API-compatible wrapper object that provides thread
safety. Returning this from api() helps keep users out of
trouble.
The changes to ThreadSafeApiCache are required to keep it API-compatible
with the original and keep tests passing. Assignments to the request_id
attribute need to be used for all future requests. It's a little unclear
if this is an intended API or just test scaffolding, but it's not too
difficult to keep working so I just did that.
Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>
diff --git a/sdk/python/arvados/api.py b/sdk/python/arvados/api.py
index 13dd8e9af..19154f3e8 100644
--- a/sdk/python/arvados/api.py
+++ b/sdk/python/arvados/api.py
@@ -374,6 +374,11 @@ def api(version=None, cache=True, host=None, token=None, insecure=False,
like you would write in user configuration; or pass additional arguments
for lower-level control over the client.
+ This function returns a `arvados.safeapi.ThreadSafeApiCache`, an
+ API-compatible wrapper around `googleapiclient.discovery.Resource`. If
+ you're handling concurrency yourself and/or your application is very
+ performance-sensitive, consider calling `api_client` directly.
+
Arguments:
version: str | None
@@ -398,21 +403,20 @@ def api(version=None, cache=True, host=None, token=None, insecure=False,
Other arguments are passed directly to `api_client`. See that function's
docstring for more information about their meaning.
"""
- if discoveryServiceUrl or host or token:
- # We pass `insecure` here for symmetry with `api_kwargs_from_config`.
- client_kwargs = normalize_api_kwargs(
- version, discoveryServiceUrl, host, token,
- insecure=insecure,
- )
- else:
- client_kwargs = api_kwargs_from_config(version)
- return api_client(
- **client_kwargs,
+ kwargs.update(
cache=cache,
+ insecure=insecure,
request_id=request_id,
timeout=timeout,
- **kwargs,
)
+ if discoveryServiceUrl or host or token:
+ kwargs.update(normalize_api_kwargs(version, discoveryServiceUrl, host, token))
+ else:
+ kwargs.update(api_kwargs_from_config(version))
+ version = kwargs.pop('version')
+ # We do the import here to avoid a circular import at the top level.
+ from .safeapi import ThreadSafeApiCache
+ return ThreadSafeApiCache({}, {}, kwargs, version)
def api_from_config(version=None, apiconfig=None, **kwargs):
"""Build an Arvados API client from a configuration mapping
@@ -421,6 +425,11 @@ def api_from_config(version=None, apiconfig=None, **kwargs):
configuration. It accepts that mapping as an argument, so you can use a
configuration that's different from what the user has set up.
+ This function returns a `arvados.safeapi.ThreadSafeApiCache`, an
+ API-compatible wrapper around `googleapiclient.discovery.Resource`. If
+ you're handling concurrency yourself and/or your application is very
+ performance-sensitive, consider calling `api_client` directly.
+
Arguments:
version: str | None
@@ -435,4 +444,4 @@ def api_from_config(version=None, apiconfig=None, **kwargs):
Other arguments are passed directly to `api_client`. See that function's
docstring for more information about their meaning.
"""
- return api_client(**api_kwargs_from_config(version, apiconfig, **kwargs))
+ return api(**api_kwargs_from_config(version, apiconfig, **kwargs))
diff --git a/sdk/python/arvados/safeapi.py b/sdk/python/arvados/safeapi.py
index 0513c7023..e9dde1962 100644
--- a/sdk/python/arvados/safeapi.py
+++ b/sdk/python/arvados/safeapi.py
@@ -15,6 +15,7 @@ import threading
from . import api
from . import config
from . import keep
+from . import util
class ThreadSafeApiCache(object):
"""Thread-safe wrapper for an Arvados API client
@@ -53,13 +54,18 @@ class ThreadSafeApiCache(object):
else:
self._api_kwargs = api.normalize_api_kwargs(version, **api_params)
self.api_token = self._api_kwargs['token']
+ self.request_id = self._api_kwargs.get('request_id')
self.local = threading.local()
self.keep = keep.KeepClient(api_client=self, **keep_params)
def localapi(self):
- if 'api' not in self.local.__dict__:
- self.local.api = api.api_client(**self._api_kwargs)
- return self.local.api
+ try:
+ client = self.local.api
+ except AttributeError:
+ client = api.api_client(**self._api_kwargs)
+ client._http._request_id = lambda: self.request_id or util.new_request_id()
+ self.local.api = client
+ return client
def __getattr__(self, name):
# Proxy nonexistent attributes to the thread-local API client.
diff --git a/sdk/python/tests/test_api.py b/sdk/python/tests/test_api.py
index c249f46d3..03af8ce5e 100644
--- a/sdk/python/tests/test_api.py
+++ b/sdk/python/tests/test_api.py
@@ -139,6 +139,27 @@ class ArvadosApiTest(run_test_server.TestCaseWithServers):
result = api.humans().get(uuid='test').execute()
self.assertEqual(string.hexdigits, ''.join(list(result.keys())))
+ def test_api_is_threadsafe(self):
+ api_kwargs = {
+ 'host': os.environ['ARVADOS_API_HOST'],
+ 'token': os.environ['ARVADOS_API_TOKEN'],
+ 'insecure': True,
+ }
+ config_kwargs = {'apiconfig': os.environ}
+ for api_constructor, kwargs in [
+ (arvados.api, {}),
+ (arvados.api, api_kwargs),
+ (arvados.api_from_config, {}),
+ (arvados.api_from_config, config_kwargs),
+ ]:
+ sub_kwargs = "kwargs" if kwargs else "no kwargs"
+ with self.subTest(f"{api_constructor.__name__} with {sub_kwargs}"):
+ api_client = api_constructor('v1', **kwargs)
+ self.assertTrue(hasattr(api_client, 'localapi'),
+ f"client missing localapi method")
+ self.assertTrue(hasattr(api_client, 'keep'),
+ f"client missing keep attribute")
+
class RetryREST(unittest.TestCase):
def setUp(self):
commit adcef9f939b663d88a12d5d3597c3b0184d2579f
Author: Brett Smith <brett.smith at curii.com>
Date: Mon Nov 28 16:30:28 2022 -0500
19686: Introduce low-level api_client constructor
When api() returns a ThreadSafeApiCache, we still want to provide a
mechanism to get a plain Resource object. The api_client() function is
that mechanism. It *just* builds the Resource object as api() did
before.
normalize_api_kwargs() takes the arguments passed to api() and turns
them into keyword arguments for api_client(). api_config_to_kwargs()
takes a configuration mapping nand turns them into keyword arguments for
api_client(). Both of these are small APIs, just returning one
dictionary from another.
With this reorganization, api(), api_from_config(), and
ThreadSafeApiClient() can all have simpler implementations.
Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>
diff --git a/sdk/python/arvados/api.py b/sdk/python/arvados/api.py
index 85d419758..13dd8e9af 100644
--- a/sdk/python/arvados/api.py
+++ b/sdk/python/arvados/api.py
@@ -1,6 +1,13 @@
# Copyright (C) The Arvados Authors. All rights reserved.
#
# SPDX-License-Identifier: Apache-2.0
+"""Arvados API client
+
+The code in this module builds Arvados API client objects you can use to submit
+Arvados API requests. This includes extending the underlying HTTP client with
+niceties such as caching, X-Request-Id header for tracking, and more. The main
+client constructors are `api` and `api_from_config`.
+"""
from __future__ import absolute_import
from future import standard_library
@@ -41,7 +48,7 @@ class OrderedJsonModel(apiclient.model.JsonModel):
"""Model class for JSON that preserves the contents' order.
API clients that care about preserving the order of fields in API
- server responses can use this model to do so, like this::
+ server responses can use this model to do so, like this:
from arvados.api import OrderedJsonModel
client = arvados.api('v1', ..., model=OrderedJsonModel())
@@ -167,130 +174,265 @@ def http_cache(data_type):
return None
return cache.SafeHTTPCache(path, max_age=60*60*24*2)
-def api(version=None, cache=True, host=None, token=None, insecure=False,
- request_id=None, timeout=5*60, **kwargs):
- """Return an apiclient Resources object for an Arvados instance.
-
- :version:
- A string naming the version of the Arvados API to use (for
- example, 'v1').
-
- :cache:
- Use a cache (~/.cache/arvados/discovery) for the discovery
- document.
-
- :host:
- The Arvados API server host (and optional :port) to connect to.
-
- :token:
- The authentication token to send with each API call.
-
- :insecure:
- If True, ignore SSL certificate validation errors.
-
- :timeout:
- A timeout value for http requests.
-
- :request_id:
- Default X-Request-Id header value for outgoing requests that
- don't already provide one. If None or omitted, generate a random
+def api_client(
+ version,
+ discoveryServiceUrl,
+ token,
+ *,
+ cache=True,
+ http=None,
+ insecure=False,
+ request_id=None,
+ timeout=5*60,
+ **kwargs,
+):
+ """Build an Arvados API client
+
+ This function returns a `googleapiclient.discovery.Resource` object
+ constructed from the given arguments. This is a relatively low-level
+ interface that requires all the necessary inputs as arguments. Most
+ users will prefer to use `api` which can accept more flexible inputs.
+
+ Arguments:
+
+ version: str
+ : A string naming the version of the Arvados API to use.
+
+ discoveryServiceUrl: str
+ : The URL used to discover APIs passed directly to
+ `googleapiclient.discovery.build`.
+
+ token: str
+ : The authentication token to send with each API call.
+
+ Keyword-only arguments:
+
+ cache: bool
+ : If true, loads the API discovery document from, or saves it to, a cache
+ on disk (located at `~/.cache/arvados/discovery`).
+
+ http: httplib2.Http | None
+ : The HTTP client object the API client object will use to make requests.
+ If not provided, this function will build its own to use. Either way, the
+ object will be patched as part of the build process.
+
+ insecure: bool
+ : If true, ignore SSL certificate validation errors. Default `False`.
+
+ request_id: str | None
+ : Default `X-Request-Id` header value for outgoing requests that
+ don't already provide one. If `None` or omitted, generate a random
ID. When retrying failed requests, the same ID is used on all
attempts.
- Additional keyword arguments will be passed directly to
- `apiclient_discovery.build` if a new Resource object is created.
- If the `discoveryServiceUrl` or `http` keyword arguments are
- missing, this function will set default values for them, based on
- the current Arvados configuration settings.
+ timeout: int
+ : A timeout value for HTTP requests in seconds. Default 300 (5 minutes).
+ Additional keyword arguments will be passed directly to
+ `googleapiclient.discovery.build`.
"""
-
- if not version:
- version = 'v1'
- _logger.info("Using default API version. " +
- "Call arvados.api('%s') instead." %
- version)
- if 'discoveryServiceUrl' in kwargs:
- if host:
- raise ValueError("both discoveryServiceUrl and host provided")
- # Here we can't use a token from environment, config file,
- # etc. Those probably have nothing to do with the host
- # provided by the caller.
- if not token:
- raise ValueError("discoveryServiceUrl provided, but token missing")
- elif host and token:
- pass
- elif not host and not token:
- return api_from_config(
- version=version, cache=cache, timeout=timeout,
- request_id=request_id, **kwargs)
- else:
- # Caller provided one but not the other
- if not host:
- raise ValueError("token argument provided, but host missing.")
- else:
- raise ValueError("host argument provided, but token missing.")
-
- if host:
- # Caller wants us to build the discoveryServiceUrl
- kwargs['discoveryServiceUrl'] = (
- 'https://%s/discovery/v1/apis/{api}/{apiVersion}/rest' % (host,))
-
- if 'http' not in kwargs:
- http_kwargs = {'ca_certs': util.ca_certs_path()}
- if cache:
- http_kwargs['cache'] = http_cache('discovery')
- if insecure:
- http_kwargs['disable_ssl_certificate_validation'] = True
- kwargs['http'] = httplib2.Http(**http_kwargs)
-
- if kwargs['http'].timeout is None:
- kwargs['http'].timeout = timeout
-
- kwargs['http'] = _patch_http_request(kwargs['http'], token)
-
- svc = apiclient_discovery.build('arvados', version, cache_discovery=False, **kwargs)
+ if http is None:
+ http = httplib2.Http(
+ ca_certs=util.ca_certs_path(),
+ cache=http_cache('discovery') if cache else None,
+ disable_ssl_certificate_validation=bool(insecure),
+ )
+ if http.timeout is None:
+ http.timeout = timeout
+ http = _patch_http_request(http, token)
+
+ svc = apiclient_discovery.build(
+ 'arvados', version,
+ cache_discovery=False,
+ discoveryServiceUrl=discoveryServiceUrl,
+ http=http,
+ **kwargs,
+ )
svc.api_token = token
svc.insecure = insecure
svc.request_id = request_id
svc.config = lambda: util.get_config_once(svc)
svc.vocabulary = lambda: util.get_vocabulary_once(svc)
svc.close_connections = types.MethodType(_close_connections, svc)
- kwargs['http'].max_request_size = svc._rootDesc.get('maxRequestSize', 0)
- kwargs['http'].cache = None
- kwargs['http']._request_id = lambda: svc.request_id or util.new_request_id()
+ http.max_request_size = svc._rootDesc.get('maxRequestSize', 0)
+ http.cache = None
+ http._request_id = lambda: svc.request_id or util.new_request_id()
return svc
-def api_from_config(version=None, apiconfig=None, **kwargs):
- """Return an apiclient Resources object enabling access to an Arvados server
- instance.
+def normalize_api_kwargs(
+ version=None,
+ discoveryServiceUrl=None,
+ host=None,
+ token=None,
+ **kwargs,
+):
+ """Validate kwargs from `api` and build kwargs for `api_client`
- :version:
- A string naming the version of the Arvados REST API to use (for
- example, 'v1').
+ This method takes high-level keyword arguments passed to the `api`
+ constructor and normalizes them into a new dictionary that can be passed
+ as keyword arguments to `api_client`. It raises `ValueError` if required
+ arguments are missing or conflict.
- :apiconfig:
- If provided, this should be a dict-like object (must support the get()
- method) with entries for ARVADOS_API_HOST, ARVADOS_API_TOKEN, and
- optionally ARVADOS_API_HOST_INSECURE. If not provided, use
- arvados.config (which gets these parameters from the environment by
- default.)
+ Arguments:
- Other keyword arguments such as `cache` will be passed along `api()`
+ version: str | None
+ : A string naming the version of the Arvados API to use. If not specified,
+ the code will log a warning and fall back to 'v1'.
+ discoveryServiceUrl: str | None
+ : The URL used to discover APIs passed directly to
+ `googleapiclient.discovery.build`. It is an error to pass both
+ `discoveryServiceUrl` and `host`.
+
+ host: str | None
+ : The hostname and optional port number of the Arvados API server. Used to
+ build `discoveryServiceUrl`. It is an error to pass both
+ `discoveryServiceUrl` and `host`.
+
+ token: str
+ : The authentication token to send with each API call.
+
+ Additional keyword arguments will be included in the return value.
+ """
+ if discoveryServiceUrl and host:
+ raise ValueError("both discoveryServiceUrl and host provided")
+ elif discoveryServiceUrl:
+ url_src = "discoveryServiceUrl"
+ elif host:
+ url_src = "host argument"
+ discoveryServiceUrl = 'https://%s/discovery/v1/apis/{api}/{apiVersion}/rest' % (host,)
+ elif token:
+ # This specific error message gets priority for backwards compatibility.
+ raise ValueError("token argument provided, but host missing.")
+ else:
+ raise ValueError("neither discoveryServiceUrl nor host provided")
+ if not token:
+ raise ValueError("%s provided, but token missing" % (url_src,))
+ if not version:
+ version = 'v1'
+ _logger.info(
+ "Using default API version. Call arvados.api(%r) instead.",
+ version,
+ )
+ return {
+ 'discoveryServiceUrl': discoveryServiceUrl,
+ 'token': token,
+ 'version': version,
+ **kwargs,
+ }
+
+def api_kwargs_from_config(version=None, apiconfig=None, **kwargs):
+ """Build `api_client` keyword arguments from configuration
+
+ This function accepts a mapping with Arvados configuration settings like
+ `ARVADOS_API_HOST` and converts them into a mapping of keyword arguments
+ that can be passed to `api_client`. If `ARVADOS_API_HOST` or
+ `ARVADOS_API_TOKEN` are not configured, it raises `ValueError`.
+
+ Arguments:
+
+ version: str | None
+ : A string naming the version of the Arvados API to use. If not specified,
+ the code will log a warning and fall back to 'v1'.
+
+ apiconfig: Mapping[str, str] | None
+ : A mapping with entries for `ARVADOS_API_HOST`, `ARVADOS_API_TOKEN`, and
+ optionally `ARVADOS_API_HOST_INSECURE`. If not provided, calls
+ `arvados.config.settings` to get these parameters from user configuration.
+
+ Additional keyword arguments will be included in the return value.
"""
- # Load from user configuration or environment
if apiconfig is None:
apiconfig = config.settings()
+ missing = " and ".join(
+ key
+ for key in ['ARVADOS_API_HOST', 'ARVADOS_API_TOKEN']
+ if key not in apiconfig
+ )
+ if missing:
+ raise ValueError(
+ "%s not set.\nPlease set in %s or export environment variable." %
+ (missing, config.default_config_file),
+ )
+ return normalize_api_kwargs(
+ version,
+ None,
+ apiconfig['ARVADOS_API_HOST'],
+ apiconfig['ARVADOS_API_TOKEN'],
+ insecure=config.flag_is_true('ARVADOS_API_HOST_INSECURE', apiconfig),
+ **kwargs,
+ )
+
+def api(version=None, cache=True, host=None, token=None, insecure=False,
+ request_id=None, timeout=5*60, *,
+ discoveryServiceUrl=None, **kwargs):
+ """Dynamically build an Arvados API client
+
+ This function provides a high-level "do what I mean" interface to build an
+ Arvados API client object. You can call it with no arguments to build a
+ client from user configuration; pass `host` and `token` arguments just
+ like you would write in user configuration; or pass additional arguments
+ for lower-level control over the client.
+
+ Arguments:
+
+ version: str | None
+ : A string naming the version of the Arvados API to use. If not specified,
+ the code will log a warning and fall back to 'v1'.
+
+ host: str | None
+ : The hostname and optional port number of the Arvados API server.
+
+ token: str | None
+ : The authentication token to send with each API call.
+
+ discoveryServiceUrl: str | None
+ : The URL used to discover APIs passed directly to
+ `googleapiclient.discovery.build`.
+
+ If `host`, `token`, and `discoveryServiceUrl` are all omitted, `host` and
+ `token` will be loaded from the user's configuration. Otherwise, you must
+ pass `token` and one of `host` or `discoveryServiceUrl`. It is an error to
+ pass both `host` and `discoveryServiceUrl`.
- errors = []
- for x in ['ARVADOS_API_HOST', 'ARVADOS_API_TOKEN']:
- if x not in apiconfig:
- errors.append(x)
- if errors:
- raise ValueError(" and ".join(errors)+" not set.\nPlease set in %s or export environment variable." % config.default_config_file)
- host = apiconfig.get('ARVADOS_API_HOST')
- token = apiconfig.get('ARVADOS_API_TOKEN')
- insecure = config.flag_is_true('ARVADOS_API_HOST_INSECURE', apiconfig)
-
- return api(version=version, host=host, token=token, insecure=insecure, **kwargs)
+ Other arguments are passed directly to `api_client`. See that function's
+ docstring for more information about their meaning.
+ """
+ if discoveryServiceUrl or host or token:
+ # We pass `insecure` here for symmetry with `api_kwargs_from_config`.
+ client_kwargs = normalize_api_kwargs(
+ version, discoveryServiceUrl, host, token,
+ insecure=insecure,
+ )
+ else:
+ client_kwargs = api_kwargs_from_config(version)
+ return api_client(
+ **client_kwargs,
+ cache=cache,
+ request_id=request_id,
+ timeout=timeout,
+ **kwargs,
+ )
+
+def api_from_config(version=None, apiconfig=None, **kwargs):
+ """Build an Arvados API client from a configuration mapping
+
+ This function builds an Arvados API client from a mapping with user
+ configuration. It accepts that mapping as an argument, so you can use a
+ configuration that's different from what the user has set up.
+
+ Arguments:
+
+ version: str | None
+ : A string naming the version of the Arvados API to use. If not specified,
+ the code will log a warning and fall back to 'v1'.
+
+ apiconfig: Mapping[str, str] | None
+ : A mapping with entries for `ARVADOS_API_HOST`, `ARVADOS_API_TOKEN`, and
+ optionally `ARVADOS_API_HOST_INSECURE`. If not provided, calls
+ `arvados.config.settings` to get these parameters from user configuration.
+
+ Other arguments are passed directly to `api_client`. See that function's
+ docstring for more information about their meaning.
+ """
+ return api_client(**api_kwargs_from_config(version, apiconfig, **kwargs))
diff --git a/sdk/python/arvados/safeapi.py b/sdk/python/arvados/safeapi.py
index c6e17cae0..0513c7023 100644
--- a/sdk/python/arvados/safeapi.py
+++ b/sdk/python/arvados/safeapi.py
@@ -1,47 +1,66 @@
# Copyright (C) The Arvados Authors. All rights reserved.
#
# SPDX-License-Identifier: Apache-2.0
+"""Thread-safe wrapper for an Arvados API client
+
+This module provides `ThreadSafeApiCache`, a thread-safe, API-compatible
+Arvados API client.
+"""
from __future__ import absolute_import
from builtins import object
-import copy
import threading
-import arvados
-import arvados.keep as keep
-import arvados.config as config
+from . import api
+from . import config
+from . import keep
class ThreadSafeApiCache(object):
- """Threadsafe wrapper for API objects.
+ """Thread-safe wrapper for an Arvados API client
- This stores and returns a different api object per thread, because httplib2
- which underlies apiclient is not threadsafe.
+ This class takes all the arguments necessary to build a lower-level
+ Arvados API client `googleapiclient.discovery.Resource`, then
+ transparently builds and wraps a unique object per thread. This works
+ around the fact that the client's underlying HTTP client object is not
+ thread-safe.
- """
+ Arguments:
- def __init__(self, apiconfig=None, keep_params={}, api_params={}):
- if apiconfig is None:
- apiconfig = config.settings()
- self.apiconfig = copy.copy(apiconfig)
- self.api_params = api_params
- self.local = threading.local()
+ apiconfig: Mapping[str, str] | None
+ : A mapping with entries for `ARVADOS_API_HOST`, `ARVADOS_API_TOKEN`,
+ and optionally `ARVADOS_API_HOST_INSECURE`. If not provided, uses
+ `arvados.config.settings` to get these parameters from user
+ configuration. You can pass an empty mapping to build the client
+ solely from `api_params`.
+
+ keep_params: Mapping[str, Any]
+ : Keyword arguments used to construct an associated
+ `arvados.keep.KeepClient`.
- # Initialize an API object for this thread before creating
- # KeepClient, this will report if ARVADOS_API_HOST or
- # ARVADOS_API_TOKEN are missing.
- self.localapi()
+ api_params: Mapping[str, Any]
+ : Keyword arguments used to construct each thread's API client. These
+ have the same meaning as in the `arvados.api.api` function.
+ version: str | None
+ : A string naming the version of the Arvados API to use. If not specified,
+ the code will log a warning and fall back to 'v1'.
+ """
+
+ def __init__(self, apiconfig=None, keep_params={}, api_params={}, version=None):
+ if apiconfig or apiconfig is None:
+ self._api_kwargs = api.api_kwargs_from_config(version, apiconfig, **api_params)
+ else:
+ self._api_kwargs = api.normalize_api_kwargs(version, **api_params)
+ self.api_token = self._api_kwargs['token']
+ self.local = threading.local()
self.keep = keep.KeepClient(api_client=self, **keep_params)
def localapi(self):
if 'api' not in self.local.__dict__:
- self.local.api = arvados.api_from_config('v1', apiconfig=self.apiconfig,
- **self.api_params)
+ self.local.api = api.api_client(**self._api_kwargs)
return self.local.api
def __getattr__(self, name):
# Proxy nonexistent attributes to the thread-local API client.
- if name == "api_token":
- return self.apiconfig['ARVADOS_API_TOKEN']
return getattr(self.localapi(), name)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list