[arvados] updated: 2.1.0-3168-g817132887
git repository hosting
git at public.arvados.org
Thu Dec 8 16:06:33 UTC 2022
Summary of changes:
sdk/cwl/arvados_cwl/__init__.py | 4 +-
sdk/python/arvados/api.py | 365 +++++++++++++++++++++++----------
sdk/python/arvados/collection.py | 2 +-
sdk/python/arvados/safeapi.py | 73 ++++---
sdk/python/tests/test_api.py | 203 +++++++++++++++++-
sdk/python/tests/test_arv_put.py | 1 +
sdk/python/tests/test_safeapi.py | 63 ++++++
services/fuse/arvados_fuse/command.py | 4 +-
services/fuse/tests/mount_test_base.py | 6 +-
services/fuse/tests/test_mount.py | 12 +-
10 files changed, 594 insertions(+), 139 deletions(-)
create mode 100644 sdk/python/tests/test_safeapi.py
via 8171328873751d5bfd47cd9da3f6ff9a66c84659 (commit)
via 71a0b1d3e4313b4ae4daf28330a2a075d30ed636 (commit)
via 3082625157ad8abf2de8b6797d41dccb1a1816ff (commit)
via 5c79b60beb2f9bbfa1234ace95979343e40c13d1 (commit)
via 2668643b7570db96651466250e7a496184f6ef0a (commit)
via adcef9f939b663d88a12d5d3597c3b0184d2579f (commit)
from 2b4dde152d81ff5946ff21553aa4e072d1f15532 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 8171328873751d5bfd47cd9da3f6ff9a66c84659
Merge: 2b4dde152 71a0b1d3e
Author: Brett Smith <brett.smith at curii.com>
Date: Thu Dec 8 11:05:57 2022 -0500
Merge branch '19686-threadsafe-api-default'
Closes #19686.
Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>
commit 71a0b1d3e4313b4ae4daf28330a2a075d30ed636
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 3082625157ad8abf2de8b6797d41dccb1a1816ff
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 5c79b60beb2f9bbfa1234ace95979343e40c13d1
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..a41219e9c
--- /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
+ expected_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)
+ keep_params = dict(expected_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')
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list