[ARVADOS] created: 1.1.1-163-g4453022
Git user
git at public.curoverse.com
Fri Dec 8 13:34:33 EST 2017
at 4453022a516e2b1deb30a71d8ee811d6593c44c3 (commit)
commit 4453022a516e2b1deb30a71d8ee811d6593c44c3
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Fri Dec 8 13:01:54 2017 -0500
12167: Test X-Request-Id request headers in Keep get/put/head reqs.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/sdk/python/tests/test_keep_client.py b/sdk/python/tests/test_keep_client.py
index c2c4728..d544b92 100644
--- a/sdk/python/tests/test_keep_client.py
+++ b/sdk/python/tests/test_keep_client.py
@@ -23,6 +23,7 @@ import urllib.parse
import arvados
import arvados.retry
+import arvados.util
from . import arvados_testutil as tutil
from . import keepstub
from . import run_test_server
@@ -518,6 +519,77 @@ class KeepClientServiceTestCase(unittest.TestCase, tutil.ApiClientMock):
@tutil.skip_sleep
+class KeepXRequestId(unittest.TestCase, tutil.ApiClientMock):
+ def setUp(self):
+ self.api_client = self.mock_keep_services(count=2)
+ self.keep_client = arvados.KeepClient(api_client=self.api_client)
+ self.data = b'xyzzy'
+ self.locator = '1271ed5ef305aadabc605b1609e24c52'
+ self.test_id = arvados.util.new_request_id()
+ self.assertRegex(self.test_id, r'^req-[a-z0-9]{20}$')
+ # If we don't set request_id to None explicitly here, it will
+ # return <MagicMock name='api_client_mock.request_id'
+ # id='123456789'>:
+ self.api_client.request_id = None
+
+ def test_default_to_api_client_request_id(self):
+ self.api_client.request_id = self.test_id
+ with tutil.mock_keep_responses(self.locator, 200, 200) as mock:
+ self.keep_client.put(self.data)
+ self.assertEqual(2, len(mock.responses))
+ for resp in mock.responses:
+ self.assertProvidedRequestId(resp)
+
+ with tutil.mock_keep_responses(self.data, 200) as mock:
+ self.keep_client.get(self.locator)
+ self.assertProvidedRequestId(mock.responses[0])
+
+ with tutil.mock_keep_responses(b'', 200) as mock:
+ self.keep_client.head(self.locator)
+ self.assertProvidedRequestId(mock.responses[0])
+
+ def test_explicit_request_id(self):
+ with tutil.mock_keep_responses(self.locator, 200, 200) as mock:
+ self.keep_client.put(self.data, request_id=self.test_id)
+ self.assertEqual(2, len(mock.responses))
+ for resp in mock.responses:
+ self.assertProvidedRequestId(resp)
+
+ with tutil.mock_keep_responses(self.data, 200) as mock:
+ self.keep_client.get(self.locator, request_id=self.test_id)
+ self.assertProvidedRequestId(mock.responses[0])
+
+ with tutil.mock_keep_responses(b'', 200) as mock:
+ self.keep_client.head(self.locator, request_id=self.test_id)
+ self.assertProvidedRequestId(mock.responses[0])
+
+ def test_automatic_request_id(self):
+ with tutil.mock_keep_responses(self.locator, 200, 200) as mock:
+ self.keep_client.put(self.data)
+ self.assertEqual(2, len(mock.responses))
+ for resp in mock.responses:
+ self.assertAutomaticRequestId(resp)
+
+ with tutil.mock_keep_responses(self.data, 200) as mock:
+ self.keep_client.get(self.locator)
+ self.assertAutomaticRequestId(mock.responses[0])
+
+ with tutil.mock_keep_responses(b'', 200) as mock:
+ self.keep_client.head(self.locator)
+ self.assertAutomaticRequestId(mock.responses[0])
+
+ def assertAutomaticRequestId(self, resp):
+ hdr = [x for x in resp.getopt(pycurl.HTTPHEADER)
+ if x.startswith('X-Request-Id: ')][0]
+ self.assertNotEqual(hdr, 'X-Request-Id: '+self.test_id)
+ self.assertRegex(hdr, r'^X-Request-Id: req-[a-z0-9]{20}$')
+
+ def assertProvidedRequestId(self, resp):
+ self.assertIn('X-Request-Id: '+self.test_id,
+ resp.getopt(pycurl.HTTPHEADER))
+
+
+ at tutil.skip_sleep
class KeepClientRendezvousTestCase(unittest.TestCase, tutil.ApiClientMock):
def setUp(self):
commit 9674a836236a91e820fd4faff32442a087501383
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Fri Dec 8 11:09:13 2017 -0500
12167: Use one X-Request-Id per arv-put; display it if not --silent.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/sdk/python/arvados/commands/put.py b/sdk/python/arvados/commands/put.py
index ec4ae8f..8bfe520 100644
--- a/sdk/python/arvados/commands/put.py
+++ b/sdk/python/arvados/commands/put.py
@@ -398,7 +398,7 @@ class ArvPutUploadJob(object):
}
def __init__(self, paths, resume=True, use_cache=True, reporter=None,
- name=None, owner_uuid=None,
+ name=None, owner_uuid=None, api_client=None,
ensure_unique_name=False, num_retries=None,
put_threads=None, replication_desired=None,
filename=None, update_time=60.0, update_collection=None,
@@ -421,6 +421,7 @@ class ArvPutUploadJob(object):
self.replication_desired = replication_desired
self.put_threads = put_threads
self.filename = filename
+ self._api_client = api_client
self._state_lock = threading.Lock()
self._state = None # Previous run state (file list & manifest)
self._current_files = [] # Current run file list
@@ -775,7 +776,8 @@ class ArvPutUploadJob(object):
if update_collection and re.match(arvados.util.collection_uuid_pattern,
update_collection):
try:
- self._remote_collection = arvados.collection.Collection(update_collection)
+ self._remote_collection = arvados.collection.Collection(
+ update_collection, api_client=self._api_client)
except arvados.errors.ApiError as error:
raise CollectionUpdateError("Cannot read collection {} ({})".format(update_collection, error))
else:
@@ -822,7 +824,11 @@ class ArvPutUploadJob(object):
# No cache file, set empty state
self._state = copy.deepcopy(self.EMPTY_STATE)
# Load the previous manifest so we can check if files were modified remotely.
- self._local_collection = arvados.collection.Collection(self._state['manifest'], replication_desired=self.replication_desired, put_threads=self.put_threads)
+ self._local_collection = arvados.collection.Collection(
+ self._state['manifest'],
+ replication_desired=self.replication_desired,
+ put_threads=self.put_threads,
+ api_client=self._api_client)
def collection_file_paths(self, col, path_prefix='.'):
"""Return a list of file paths by recursively go through the entire collection `col`"""
@@ -977,8 +983,12 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
else:
logger.setLevel(logging.INFO)
status = 0
+
+ request_id = arvados.util.new_request_id()
+ logger.info('X-Request-Id: '+request_id)
+
if api_client is None:
- api_client = arvados.api('v1')
+ api_client = arvados.api('v1', request_id=request_id)
# Determine the name to use
if args.name:
@@ -1063,6 +1073,7 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
use_cache = args.use_cache,
filename = args.filename,
reporter = reporter,
+ api_client = api_client,
num_retries = args.retries,
replication_desired = args.replication,
put_threads = args.threads,
commit e13f1473fb0ea51414bc54136723ec3acd7045f2
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Fri Dec 8 10:16:08 2017 -0500
12167: Use one X-Request-Id per arv-get process; display it if -v.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/sdk/python/arvados/commands/get.py b/sdk/python/arvados/commands/get.py
index 881fdd6..1e52714 100755
--- a/sdk/python/arvados/commands/get.py
+++ b/sdk/python/arvados/commands/get.py
@@ -81,6 +81,10 @@ Overwrite existing files while writing. The default behavior is to
refuse to write *anything* if any of the output files already
exist. As a special case, -f is not needed to write to stdout.
""")
+group.add_argument('-v', action='count', default=0,
+ help="""
+Once for verbose mode, twice for debug mode.
+""")
group.add_argument('--skip-existing', action='store_true',
help="""
Skip files that already exist. The default behavior is to refuse to
@@ -140,8 +144,13 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
stdout = stdout.buffer
args = parse_arguments(arguments, stdout, stderr)
+ logger.setLevel(logging.WARNING - 10 * args.v)
+
+ request_id = arvados.util.new_request_id()
+ logger.info('X-Request-Id: '+request_id)
+
if api_client is None:
- api_client = arvados.api('v1')
+ api_client = arvados.api('v1', request_id=request_id)
r = re.search(r'^(.*?)(/.*)?$', args.locator)
col_loc = r.group(1)
@@ -157,14 +166,15 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
open_flags |= os.O_EXCL
try:
if args.destination == "-":
- write_block_or_manifest(dest=stdout, src=col_loc,
- api_client=api_client, args=args)
+ write_block_or_manifest(
+ dest=stdout, src=col_loc,
+ api_client=api_client, args=args)
else:
out_fd = os.open(args.destination, open_flags)
with os.fdopen(out_fd, 'wb') as out_file:
- write_block_or_manifest(dest=out_file,
- src=col_loc, api_client=api_client,
- args=args)
+ write_block_or_manifest(
+ dest=out_file, src=col_loc,
+ api_client=api_client, args=args)
except (IOError, OSError) as error:
logger.error("can't write to '{}': {}".format(args.destination, error))
return 1
@@ -180,7 +190,8 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
return 0
try:
- reader = arvados.CollectionReader(col_loc, num_retries=args.retries)
+ reader = arvados.CollectionReader(
+ col_loc, api_client=api_client, num_retries=args.retries)
except Exception as error:
logger.error("failed to read collection: {}".format(error))
return 1
@@ -305,5 +316,6 @@ def write_block_or_manifest(dest, src, api_client, args):
dest.write(kc.get(src, num_retries=args.retries))
else:
# collection UUID or portable data hash
- reader = arvados.CollectionReader(src, num_retries=args.retries)
+ reader = arvados.CollectionReader(
+ src, api_client=api_client, num_retries=args.retries)
dest.write(reader.manifest_text(strip=args.strip_manifest).encode())
commit 2321fa1ced5faa1ace9d5b72e3b2eb4afd0e721f
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Fri Dec 8 10:13:27 2017 -0500
12167: Send caller-provided or random X-Request-Id in Keep requests.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py
index e6e93f0..351f7f5 100644
--- a/sdk/python/arvados/keep.py
+++ b/sdk/python/arvados/keep.py
@@ -291,7 +291,8 @@ class KeepClient(object):
def __init__(self, root, user_agent_pool=queue.LifoQueue(),
upload_counter=None,
- download_counter=None, **headers):
+ download_counter=None,
+ headers={}):
self.root = root
self._user_agent_pool = user_agent_pool
self._result = {'error': None}
@@ -920,7 +921,7 @@ class KeepClient(object):
_logger.debug("{}: {}".format(locator, sorted_roots))
return sorted_roots
- def map_new_services(self, roots_map, locator, force_rebuild, need_writable, **headers):
+ def map_new_services(self, roots_map, locator, force_rebuild, need_writable, headers):
# roots_map is a dictionary, mapping Keep service root strings
# to KeepService objects. Poll for Keep services, and add any
# new ones to roots_map. Return the current list of local
@@ -933,7 +934,7 @@ class KeepClient(object):
root, self._user_agent_pool,
upload_counter=self.upload_counter,
download_counter=self.download_counter,
- **headers)
+ headers=headers)
return local_roots
@staticmethod
@@ -963,14 +964,14 @@ class KeepClient(object):
return None
@retry.retry_method
- def head(self, loc_s, num_retries=None):
- return self._get_or_head(loc_s, method="HEAD", num_retries=num_retries)
+ def head(self, loc_s, **kwargs):
+ return self._get_or_head(loc_s, method="HEAD", **kwargs)
@retry.retry_method
- def get(self, loc_s, num_retries=None):
- return self._get_or_head(loc_s, method="GET", num_retries=num_retries)
+ def get(self, loc_s, **kwargs):
+ return self._get_or_head(loc_s, method="GET", **kwargs)
- def _get_or_head(self, loc_s, method="GET", num_retries=None):
+ def _get_or_head(self, loc_s, method="GET", num_retries=None, request_id=None):
"""Get data from Keep.
This method fetches one or more blocks of data from Keep. It
@@ -1005,6 +1006,12 @@ class KeepClient(object):
self.misses_counter.add(1)
+ headers = {
+ 'X-Request-Id': (request_id or
+ (hasattr(self, 'api_client') and self.api_client.request_id) or
+ arvados.util.new_request_id()),
+ }
+
# If the locator has hints specifying a prefix (indicating a
# remote keepproxy) or the UUID of a local gateway service,
# read data from the indicated service(s) instead of the usual
@@ -1021,7 +1028,8 @@ class KeepClient(object):
roots_map = {
root: self.KeepService(root, self._user_agent_pool,
upload_counter=self.upload_counter,
- download_counter=self.download_counter)
+ download_counter=self.download_counter,
+ headers=headers)
for root in hint_roots
}
@@ -1040,7 +1048,8 @@ class KeepClient(object):
sorted_roots = self.map_new_services(
roots_map, locator,
force_rebuild=(tries_left < num_retries),
- need_writable=False)
+ need_writable=False,
+ headers=headers)
except Exception as error:
loop.save_result(error)
continue
@@ -1084,7 +1093,7 @@ class KeepClient(object):
"failed to read {}".format(loc_s), service_errors, label="service")
@retry.retry_method
- def put(self, data, copies=2, num_retries=None):
+ def put(self, data, copies=2, num_retries=None, request_id=None):
"""Save data in Keep.
This method will get a list of Keep services from the API server, and
@@ -1114,9 +1123,12 @@ class KeepClient(object):
return loc_s
locator = KeepLocator(loc_s)
- headers = {}
- # Tell the proxy how many copies we want it to store
- headers['X-Keep-Desired-Replicas'] = str(copies)
+ headers = {
+ 'X-Request-Id': (request_id or
+ (hasattr(self, 'api_client') and self.api_client.request_id) or
+ arvados.util.new_request_id()),
+ 'X-Keep-Desired-Replicas': str(copies),
+ }
roots_map = {}
loop = retry.RetryLoop(num_retries, self._check_loop_result,
backoff_start=2)
@@ -1125,7 +1137,9 @@ class KeepClient(object):
try:
sorted_roots = self.map_new_services(
roots_map, locator,
- force_rebuild=(tries_left < num_retries), need_writable=True, **headers)
+ force_rebuild=(tries_left < num_retries),
+ need_writable=True,
+ headers=headers)
except Exception as error:
loop.save_result(error)
continue
commit 15f50edccc93c0c97eef30ff2662ab797a670e92
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Fri Dec 8 10:11:21 2017 -0500
12167: Send caller-provided or random X-Request-Id in API requests.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/sdk/python/arvados/api.py b/sdk/python/arvados/api.py
index e69f1a1..4611a1a 100644
--- a/sdk/python/arvados/api.py
+++ b/sdk/python/arvados/api.py
@@ -52,19 +52,18 @@ class OrderedJsonModel(apiclient.model.JsonModel):
return body
-def _intercept_http_request(self, uri, method="GET", **kwargs):
+def _intercept_http_request(self, uri, method="GET", headers={}, **kwargs):
if (self.max_request_size and
kwargs.get('body') and
self.max_request_size < len(kwargs['body'])):
raise apiclient_errors.MediaUploadSizeError("Request size %i bytes exceeds published limit of %i bytes" % (len(kwargs['body']), self.max_request_size))
- if 'headers' not in kwargs:
- kwargs['headers'] = {}
-
if config.get("ARVADOS_EXTERNAL_CLIENT", "") == "true":
- kwargs['headers']['X-External-Client'] = '1'
+ headers['X-External-Client'] = '1'
- kwargs['headers']['Authorization'] = 'OAuth2 %s' % self.arvados_api_token
+ headers['Authorization'] = 'OAuth2 %s' % self.arvados_api_token
+ if not headers.get('X-Request-Id'):
+ headers['X-Request-Id'] = self._request_id()
retryable = method in [
'DELETE', 'GET', 'HEAD', 'OPTIONS', 'PUT']
@@ -83,7 +82,7 @@ def _intercept_http_request(self, uri, method="GET", **kwargs):
for _ in range(retry_count):
self._last_request_time = time.time()
try:
- return self.orig_http_request(uri, method, **kwargs)
+ return self.orig_http_request(uri, method, headers=headers, **kwargs)
except http.client.HTTPException:
_logger.debug("Retrying API request in %d s after HTTP error",
delay, exc_info=True)
@@ -101,7 +100,7 @@ def _intercept_http_request(self, uri, method="GET", **kwargs):
delay = delay * self._retry_delay_backoff
self._last_request_time = time.time()
- return self.orig_http_request(uri, method, **kwargs)
+ return self.orig_http_request(uri, method, headers=headers, **kwargs)
def _patch_http_request(http, api_token):
http.arvados_api_token = api_token
@@ -113,6 +112,7 @@ def _patch_http_request(http, api_token):
http._retry_delay_initial = RETRY_DELAY_INITIAL
http._retry_delay_backoff = RETRY_DELAY_BACKOFF
http._retry_count = RETRY_COUNT
+ http._request_id = util.new_request_id
return http
# Monkey patch discovery._cast() so objects and arrays get serialized
@@ -148,7 +148,8 @@ 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, **kwargs):
+def api(version=None, cache=True, host=None, token=None, insecure=False,
+ request_id=None, **kwargs):
"""Return an apiclient Resources object for an Arvados instance.
:version:
@@ -168,6 +169,12 @@ def api(version=None, cache=True, host=None, token=None, insecure=False, **kwarg
:insecure:
If True, ignore SSL certificate validation errors.
+ :request_id:
+ 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
@@ -192,7 +199,8 @@ def api(version=None, cache=True, host=None, token=None, insecure=False, **kwarg
elif host and token:
pass
elif not host and not token:
- return api_from_config(version=version, cache=cache, **kwargs)
+ return api_from_config(
+ version=version, cache=cache, request_id=request_id, **kwargs)
else:
# Caller provided one but not the other
if not host:
@@ -218,8 +226,10 @@ def api(version=None, cache=True, host=None, token=None, insecure=False, **kwarg
svc = apiclient_discovery.build('arvados', version, cache_discovery=False, **kwargs)
svc.api_token = token
svc.insecure = insecure
+ svc.request_id = request_id
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()
return svc
def api_from_config(version=None, apiconfig=None, **kwargs):
diff --git a/sdk/python/arvados/util.py b/sdk/python/arvados/util.py
index 1a97358..66da2d1 100644
--- a/sdk/python/arvados/util.py
+++ b/sdk/python/arvados/util.py
@@ -2,10 +2,14 @@
#
# SPDX-License-Identifier: Apache-2.0
+from __future__ import division
+from builtins import range
+
import fcntl
import hashlib
import httplib2
import os
+import random
import re
import subprocess
import errno
@@ -399,3 +403,16 @@ def ca_certs_path(fallback=httplib2.CA_CERTS):
if os.path.exists(ca_certs_path):
return ca_certs_path
return fallback
+
+def new_request_id():
+ rid = "req-"
+ # 2**104 > 36**20 > 2**103
+ n = random.getrandbits(104)
+ for _ in range(20):
+ c = n % 36
+ if c < 10:
+ rid += chr(c+ord('0'))
+ else:
+ rid += chr(c+ord('a')-10)
+ n = n // 36
+ return rid
diff --git a/sdk/python/tests/test_api.py b/sdk/python/tests/test_api.py
index b467f32..8d3142a 100644
--- a/sdk/python/tests/test_api.py
+++ b/sdk/python/tests/test_api.py
@@ -143,6 +143,33 @@ class RetryREST(unittest.TestCase):
[mock.call(RETRY_DELAY_INITIAL)])
@mock.patch('time.sleep')
+ def test_same_automatic_request_id_on_retry(self, sleep):
+ self.api._http.orig_http_request.side_effect = (
+ socket.error('mock error'),
+ self.request_success,
+ )
+ self.api.users().current().execute()
+ calls = self.api._http.orig_http_request.call_args_list
+ self.assertEqual(len(calls), 2)
+ self.assertEqual(
+ calls[0][1]['headers']['X-Request-Id'],
+ calls[1][1]['headers']['X-Request-Id'])
+ self.assertRegex(calls[0][1]['headers']['X-Request-Id'], r'^req-[a-z0-9]{20}$')
+
+ @mock.patch('time.sleep')
+ def test_provided_request_id_on_retry(self, sleep):
+ self.api.request_id='fake-request-id'
+ self.api._http.orig_http_request.side_effect = (
+ socket.error('mock error'),
+ self.request_success,
+ )
+ self.api.users().current().execute()
+ calls = self.api._http.orig_http_request.call_args_list
+ self.assertEqual(len(calls), 2)
+ for call in calls:
+ self.assertEqual(call[1]['headers']['X-Request-Id'], 'fake-request-id')
+
+ @mock.patch('time.sleep')
def test_socket_error_retry_delay(self, sleep):
self.api._http.orig_http_request.side_effect = socket.error('mock')
self.api._http._retry_count = 3
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list