launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/unbreak-request-retry into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/unbreak-request-retry/+merge/51234

The transaction retry machinery somehow calls clear_request_started() twice without set_request_started() in the middle. I don't quite know why yet, but it does, and it worked until r12418. This branch makes it work again, and adds a low-level test.

An end-to-end request retry test will hopefully come soon.
-- 
https://code.launchpad.net/~wgrant/launchpad/unbreak-request-retry/+merge/51234
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/unbreak-request-retry into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/adapter.py'
--- lib/canonical/launchpad/webapp/adapter.py	2011-02-21 17:56:57 +0000
+++ lib/canonical/launchpad/webapp/adapter.py	2011-02-25 00:01:36 +0000
@@ -198,8 +198,9 @@
     _local.request_start_time = None
     request = get_current_browser_request()
     set_request_timeline(request, Timeline())
-    transaction.manager.unregisterSynch(_local.commit_logger)
-    del _local.commit_logger
+    if getattr(_local, 'commit_logger', None) is None:
+        transaction.manager.unregisterSynch(_local.commit_logger)
+        del _local.commit_logger
 
 
 def summarize_requests():

=== modified file 'lib/canonical/launchpad/webapp/ftests/test_adapter.txt'
--- lib/canonical/launchpad/webapp/ftests/test_adapter.txt	2011-02-14 14:47:09 +0000
+++ lib/canonical/launchpad/webapp/ftests/test_adapter.txt	2011-02-25 00:01:36 +0000
@@ -128,6 +128,16 @@
     Transaction completed, status: Active
     >>> clear_request_started()
 
+While you're not meant to call clear_request_started() without having
+a request in progress, some exception handlers do. We raise a warning and let
+it pass.
+
+    >>> import warnings
+    >>> with warnings.catch_warnings(record=True) as no_request_warning:
+    ...     clear_request_started()
+    >>> print no_request_warning[0].message
+    clear_request_started() called outside of a request
+
 
 Statement Timeout
 =================