launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/python-oops-tools/amqp-polish into lp:python-oops-tools.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #884569 in python-oops-tools: "confusion when amqp2disk buffers in -v mode "
  https://bugs.launchpad.net/python-oops-tools/+bug/884569
  Bug #884571 in python-oops-tools: "amqp2disk oopses with lower case hashes are not accessible"
  https://bugs.launchpad.net/python-oops-tools/+bug/884571
  Bug #885416 in python-oops-tools: "oopses with bad tuples in req_vars fail to import"
  https://bugs.launchpad.net/python-oops-tools/+bug/885416

For more details, see:
https://code.launchpad.net/~lifeless/python-oops-tools/amqp-polish/+merge/81075

More polish for weird/broken oopses - we're getting single-tuple elements [('field.blob',)] rather than [('field.blob', 'xxxx')].

These need to be recorded rather than fail-to-import.

We could have used a marker like 'UNKNOWN CONTENTS' but meh - couldn't decide so went with less typing.
-- 
https://code.launchpad.net/~lifeless/python-oops-tools/amqp-polish/+merge/81075
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/python-oops-tools/amqp-polish into lp:python-oops-tools.

=== modified file 'src/oopstools/NEWS.txt'
--- src/oopstools/NEWS.txt	2011-11-01 03:06:49 +0000
+++ src/oopstools/NEWS.txt	2011-11-02 20:29:35 +0000
@@ -8,9 +8,12 @@
 * Flush stdout when logging an OOPS receipt in amqp2disk.
   (Robert Collins, #884569)
 
-* Force received OOPS ids to uppercase in amqp2disk to fit in with the UI
-  - otherwise they cannot be accessed.
-  (Robert Collins, #884571)
+* Force received OOPS ids to uppercase in amqp2disk and dboopsloader to fit in
+  with the UI - otherwise they cannot be accessed if they contain lowercase
+  characters. (Robert Collins, #884571)
+
+* OOPS reports that don't meet the normal rules for req_vars are handled
+  a bit better (Robert Collins, William Grant, #885416)
 
 0.6.1
 =====

=== modified file 'src/oopstools/oops/models.py'
--- src/oopstools/oops/models.py	2011-10-31 02:07:24 +0000
+++ src/oopstools/oops/models.py	2011-11-02 20:29:35 +0000
@@ -305,7 +305,16 @@
         is_local_referrer = False)
     # Grab data needed by the Oops database model from the req_vars.
     req_vars = oops.get('req_vars') or ()
-    for key, value in req_vars:
+    # Some badly written OOPSes have single item tuples. (field.blob was seen).
+    def ensure_tuple(iterable):
+        for item in iterable:
+            try:
+                key, value = item
+            except ValueError:
+                key = item[0]
+                value = ''
+            yield key, value
+    for key, value in ensure_tuple(req_vars):
         if key == 'REQUEST_METHOD':
             # Truncate the REQUEST_METHOD if it's longer than 10 characters
             # since a str longer than that is not allowed in the DB.
@@ -476,6 +485,7 @@
         oopsid = parsed_oops.get('id')
         if oopsid is None:
             raise OopsReadError('Not a valid OOPS report')
+        parsed_oops['id'] = parsed_oops['id'].upper()
         try:
             res = cls.objects.get(oopsid__exact=oopsid)
         except cls.DoesNotExist:

=== modified file 'src/oopstools/oops/test/test_dboopsloader.py'
--- src/oopstools/oops/test/test_dboopsloader.py	2011-10-31 02:07:24 +0000
+++ src/oopstools/oops/test/test_dboopsloader.py	2011-11-02 20:29:35 +0000
@@ -165,3 +165,8 @@
         oops = parsed_oops_to_model_oops(report, 'test_broken_url_handling')
         self.assertEqual(expected_url, oops.url)
 
+    def test_broken_reqvars_bug_885416(self):
+        # If the tuples in req_vars have only one element the oops still needs
+        # to be loaded - we need to tolerate some bad data.
+        report = {'req_vars': [('foo',)], 'id': 'bug-885416'}
+        oops = parsed_oops_to_model_oops(report, 'test-bug-885416')