William Grant has proposed merging lp:~wgrant/launchpad/unbreak-api-timeouts
into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #740750 in Launchpad itself: "API timeouts broken and returning no useful
data..."
https://bugs.launchpad.net/launchpad/+bug/740750
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/unbreak-api-timeouts/+merge/58226
Once upon a time, timeout page renders themselves timed out when querying the
logged in user. So RequestExpiredView.__init__ was taught to reset the
remaining time.
Somewhat later, in r7184, this was fixed for all exception types by resetting
the timeout in the publication's beginErrorHandlingTransaction. The hack in
RequestExpiredView.__init__ was now superfluous.
A month or so ago lazr.restful's exception handling changed. It now gets and
instantiates the relevant view to check if it's a webservice exception view. If
it is, it is handled internally. If not, it is raised up to the main webapp
publisher. But instantiating RequestExpiredView there erases the request
timeline before it can be saved, so API timeouts have no statement log -- bug
#740750.
This branch is just a quick fix, removing the superfluous request timeline
reset. The relevant test was also removed, since the behaviour has changed and
the global publication reset is tested in test_adapter_timeout.txt.
API and webapp timeouts and other exception types still work fine, even with
statement logs this time.
I will discuss with Zope people whether lazr.restful should be instantiating
these views, but it doesn't really matter right now as this code is redundant
anyway.
--
https://code.launchpad.net/~wgrant/launchpad/unbreak-api-timeouts/+merge/58226
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~wgrant/launchpad/unbreak-api-timeouts into lp:launchpad.
=== modified file 'lib/canonical/launchpad/webapp/error.py'
--- lib/canonical/launchpad/webapp/error.py 2011-04-06 05:40:04 +0000
+++ lib/canonical/launchpad/webapp/error.py 2011-04-19 04:27:29 +0000
@@ -241,10 +241,6 @@
# is really just a guess and I don't think any clients actually
# pay attention to it - it is just a hint.
request.response.setHeader('Retry-After', 900)
- # Reset the timeout timer, so that we can issue db queries when
- # rendering the page.
- clear_request_started()
- set_request_started()
class InvalidBatchSizeView(SystemErrorView):
=== removed file 'lib/canonical/launchpad/webapp/tests/test_request_expire_render.txt'
--- lib/canonical/launchpad/webapp/tests/test_request_expire_render.txt 2010-10-18 22:24:59 +0000
+++ lib/canonical/launchpad/webapp/tests/test_request_expire_render.txt 1970-01-01 00:00:00 +0000
@@ -1,55 +0,0 @@
-= Rendering timeout exceptions =
-
-When a page takes too long to render, a timeout exception is raised. A
-timeout exception should provide IRequestExpired.
-
- >>> from zope.interface import implements
- >>> from canonical.database.interfaces import IRequestExpired
- >>> class TimeoutException:
- ... implements(IRequestExpired)
-
-After a timeout has happened, we can no longer do any more db queries,
-since get_request_remaining_seconds will raise. However, if the user is logged
-in, a query will be issued to render the "Logged in as No Privileges Person".
-
- >>> from canonical.config import config
- >>> from textwrap import dedent
- >>> test_data = dedent("""
- ... [database]
- ... db_statement_timeout: 10000
- ... """)
- >>> config.push('test_data', test_data)
-
- >>> from time import time
- >>> from canonical.launchpad.webapp.adapter import (
- ... get_request_remaining_seconds, set_request_started)
- >>> login('[email protected]')
- >>> too_early = time() - (config.database.db_statement_timeout / 1000 + 1)
- >>> set_request_started(too_early)
- >>> get_request_remaining_seconds()
- Traceback (most recent call last):
- ...
- RequestExpired: request expired.
-
-Before the OOPS page is rendered, the timeout is reset, so that we can
-issue the DB query and render the page, showing the OOPS id to the user.
-
- >>> from canonical.launchpad.webapp.interaction import (
- ... get_current_principal)
- >>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
- >>> request = LaunchpadTestRequest(environ={'PATH_INFO': '/'})
- >>> request.oopsid = 'OOPS_ID_MARKER'
- >>> request.setPrincipal(get_current_principal())
-
- >>> from zope.component import getMultiAdapter
- >>> timeout_exception = TimeoutException()
- >>> exception_view = getMultiAdapter(
- ... (timeout_exception, request), name="index.html")
- >>> get_request_remaining_seconds() > 0
- True
- >>> print exception_view()
- <...OOPS_ID_MARKER...
-
- >>> test_config_data = config.pop('test_data')
- >>> from canonical.launchpad.webapp.adapter import clear_request_started
- >>> clear_request_started()
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
