Merge lp:~mterry/ubuntuone-couch/dynamic-timeout into lp:ubuntuone-couch

[Michael Terry]

Allow 10s per megabyte in request before timing out

I was experiencing timeouts again in Deja Dup, which tries to upload backups in 25M chunks. The previous 10s timeout wasn't cutting it.

Then I realized that the timeout should probably scale with the size of the upload. So here's a branch that gives 10s per megabyte.

[Eric Casteleijn (thisfred)]

Looks great, sorry for taking so long to respond, merge proposal got lost in the mail that my dog ate.

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~mterry/ubuntuone-couch/dynamic-timeout into lp:ubuntuone-couch failed. Below is the output from the failed tests.

Running test suite for ubuntuone
ubuntuone.tests.test_couch
  QueryTestCase
    test_get_user_info ... [OK]
    test_get_user_info_404 ... [OK]
    test_request ... [OK]
    test_request_400 ... [OK]
    test_request_401 ... [OK]
    test_request_911 ... [OK]
    test_request_all_dbs ... [OK]
    test_request_all_dbs_with_extra_headers ... [OK]
    test_request_plaintext ... [OK]
    test_request_server_override ... [OK]
twisted.trial.unittest
  TestCase
    runTest ... [OK]
ubuntuone.tests.test_auth
  QueryTestCase
    test_get_oauth_credentials ... [OK]
    test_get_oauth_request_header ... [OK]
    test_get_oauth_request_header_http ... [OK]
    test_get_oauth_token ... [OK]
    test_request ... Traceback (most recent call last):
  File "/home/otto/tarmac-builds/ubuntuone-couch/trunk/ubuntuone/tests/test_auth.py", line 128, in test_request
    result = request('https://something.one.ubuntu.com/')
  File "/home/otto/tarmac-builds/ubuntuone-couch/trunk/ubuntuone/couch/auth.py", line 144, in request
    http = httplib2.Http(timeout=timeout, disable_ssl_certificate_validation=True)
  File "/usr/lib/pymodules/python2.7/mocker.py", line 1149, in __call__
    return self.__mocker_act__("call", args, kwargs)
  File "/usr/lib/pymodules/python2.7/mocker.py", line 1115, in __mocker_act__
    return path.execute(path.root_object)
  File "/usr/lib/pymodules/python2.7/mocker.py", line 1344, in execute
    object = action.execute(object)
  File "/usr/lib/pymodules/python2.7/mocker.py", line 1270, in execute
    result = object(*self.args, **self.kwargs)
exceptions.TypeError: __init__() got an unexpected keyword argument 'disable_ssl_certificate_validation'
[ERROR]
Traceback (most recent call last):
  File "/home/otto/tarmac-builds/ubuntuone-couch/trunk/ubuntuone/tests/test_auth.py", line 47, in tearDown
    self.mocker.verify()
  File "/usr/lib/pymodules/python2.7/mocker.py", line 579, in verify
    raise AssertionError(os.linesep.join(message))
exceptions.AssertionError: [Mocker] Unmet expectations:

=> httplib2.Http(timeout=10)
 - Performed fewer times than expected.

=> httplib2.Http(timeout=10).request('https://something.one.ubuntu.com/', body=None, headers=ANY, method='GET')
 - Performed fewer times than expected.

[ERROR]
    test_request_no_oauth_credentials ... ERROR:root:No credentials found in SSO. Credentials Not Found Error: missing key consumer_key
   ...

[Michael Terry (mterry)]

Oh, sorry. I had accidentally included the contents of https://code.launchpad.net/~mterry/ubuntuone-couch/no-ssl-validation in this branch and didn't notice. Let me update this branch to remove that.

[lp:~mterry/ubuntuone-couch/dynamic-timeout]

14. By Michael Terry on 2011-08-08

dynamically increase the http timeout for larger requests

[Michael Terry (mterry)]

OK, branch updated. Please re-review.

review: Needs Resubmitting

[Michael Terry (mterry)]

Changing vote to abstain.

review: Abstain

[dobey (dobey)]

Looks like you also need to update the tests. Seems a few are expecting timeout=10 to get passed in. They should probably expect Http(timeout=ANY) (from mocker import ANY).

[ERROR]
Traceback (most recent call last):
  File "/home/otto/tarmac-builds/ubuntuone-couch/trunk/ubuntuone/tests/test_auth.py", line 47, in tearDown
    self.mocker.verify()
  File "/usr/lib/pymodules/python2.7/mocker.py", line 579, in verify
    raise AssertionError(os.linesep.join(message))
exceptions.AssertionError: [Mocker] Unmet expectations:

=> httplib2.Http(timeout=10)
 - Performed fewer times than expected.

=> httplib2.Http(timeout=10).request('https://something.one.ubuntu.com/', body=None, headers=ANY, method='GET')
 - Performed fewer times than expected.

ubuntuone.tests.test_auth.QueryTestCase.test_request
ubuntuone.tests.test_auth.QueryTestCase.test_request_plaintext

review: Needs Fixing

[lp:~mterry/ubuntuone-couch/dynamic-timeout]

15. By Michael Terry on 2011-08-09

add some more spaces to avoid lintian warnings

[dobey (dobey)]

OK. Perhaps the expectation errors were due to the ssl argument being there as well, and now that you've removed that, it's fine.

review: Approve

=== modified file 'ubuntuone/couch/auth.py'
--- ubuntuone/couch/auth.py	2011-05-25 14:04:22 +0000
+++ ubuntuone/couch/auth.py	2011-08-09 13:17:31 +0000
@@ -134,10 +134,13 @@
     for key, value in urlparse.parse_qs(query).items():
         parameters[key] = value[0]
 
+    request_len = len(request_body) if request_body else 0
+    timeout = 10 * (request_len / 1024 / 1024 + 1)  # 10 seconds per megabyte
+
     oauth_header = get_oauth_request_header(
         consumer, token, url, http_method, signature_method, parameters)
     headers = headers or {}
     headers.update(oauth_header)
-    http = httplib2.Http(timeout=10)
+    http = httplib2.Http(timeout=timeout)
     return http.request(
         url, method=http_method, headers=headers, body=request_body)