4956: Refactor http request patching used in Python SDK.

Test_request_too_large uses published size instead of hardcoded size.  Make
note that user must configure upstream web server to set request size limits.

diff --git a/sdk/python/arvados/api.py b/sdk/python/arvados/api.py
index 949ead36443eae97f34fdb6be8badfb7b3ba8248..6103babe8365db27fdd2c0b71cb54cc1ee08f66e 100644 (file)
--- a/sdk/python/arvados/api.py
+++ b/sdk/python/arvados/api.py
@@ -14,43 +14,38 @@ import util
 
 _logger = logging.getLogger('arvados.api')
 
-class CredentialsFromToken(object):
-    def __init__(self, api_token):
-        self.api_token = api_token
-
-    @staticmethod
-    def http_request(self, uri, **kwargs):
-        from httplib import BadStatusLine
-
-        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'
-
-        kwargs['headers']['Authorization'] = 'OAuth2 %s' % self.arvados_api_token
-        try:
-            return self.orig_http_request(uri, **kwargs)
-        except BadStatusLine:
-            # This is how httplib tells us that it tried to reuse an
-            # existing connection but it was already closed by the
-            # server. In that case, yes, we would like to retry.
-            # Unfortunately, we are not absolutely certain that the
-            # previous call did not succeed, so this is slightly
-            # risky.
-            return self.orig_http_request(uri, **kwargs)
-
-    def authorize(self, http):
-        http.arvados_api_token = self.api_token
-        http.orig_http_request = http.request
-        http.request = types.MethodType(self.http_request, http)
-        http.max_request_size = 0
-        return http
+def intercept_http_request(self, uri, **kwargs):
+    from httplib import BadStatusLine
+
+    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'
+
+    kwargs['headers']['Authorization'] = 'OAuth2 %s' % self.arvados_api_token
+    try:
+        return self.orig_http_request(uri, **kwargs)
+    except BadStatusLine:
+        # This is how httplib tells us that it tried to reuse an
+        # existing connection but it was already closed by the
+        # server. In that case, yes, we would like to retry.
+        # Unfortunately, we are not absolutely certain that the
+        # previous call did not succeed, so this is slightly
+        # risky.
+        return self.orig_http_request(uri, **kwargs)
+
+def patch_http_request(http, api_token):
+    http.arvados_api_token = api_token
+    http.max_request_size = 0
+    http.orig_http_request = http.request
+    http.request = types.MethodType(intercept_http_request, http)
+    return http
 
 # Monkey patch discovery._cast() so objects and arrays get serialized
 # with json.dumps() instead of str().
@@ -150,8 +145,7 @@ def api(version=None, cache=True, host=None, token=None, insecure=False, **kwarg
             http_kwargs['disable_ssl_certificate_validation'] = True
         kwargs['http'] = httplib2.Http(**http_kwargs)
 
-    credentials = CredentialsFromToken(api_token=token)
-    kwargs['http'] = credentials.authorize(kwargs['http'])
+    kwargs['http'] = patch_http_request(kwargs['http'], token)
 
     svc = apiclient_discovery.build('arvados', version, **kwargs)
     svc.api_token = token

diff --git a/sdk/python/tests/test_api.py b/sdk/python/tests/test_api.py
index 1080b3c859d3337fe5bd03ff98d2bb8f992c3a94..faaaac307cf893398875d0aa5296e3fa709dd5fb 100644 (file)
--- a/sdk/python/tests/test_api.py
+++ b/sdk/python/tests/test_api.py
@@ -101,8 +101,10 @@ class ArvadosApiClientTest(unittest.TestCase):
         self.assertIn("500", str(err_ctx.exception))
 
     def test_request_too_large(self):
+        api = arvados.api('v1')
+        maxsize = api._rootDesc.get('maxRequestSize', 0)
         with self.assertRaises(apiclient_errors.MediaUploadSizeError):
-            text = "X" * (128 * 1024 * 1024)
+            text = "X" * maxsize
             arvados.api('v1').collections().create(body={"manifest_text": text}).execute()
 
 

diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index 952ed2e852f626e1afc30e180d5645c87511e884..5547704fc388fd46719095e5b67f5bf339835ff0 100644 (file)
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -250,6 +250,8 @@ common:
   # collection's replication_desired attribute is nil.
   default_collection_replication: 2
 
-  # Maximum size (in bytes) allowed for a single API request.  Is included in
-  # the discovery document for use by clients.
+  # Maximum size (in bytes) allowed for a single API request that will be
+  # published in the discovery document for use by clients.
+  # Note you must separately configure the upstream web server or proxy to
+  # actually enforce the desired maximum request size on the server side.
   max_request_size: 134217728