launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #888866 in python-oops-tools: "req_vars is documented as a list of tuples"
  https://bugs.launchpad.net/python-oops-tools/+bug/888866

For more details, see:
https://code.launchpad.net/~lifeless/python-oops-tools/bug-888866/+merge/81933

First step in moving req_vars to be dicts. See the linked bug for more details.
-- 
https://code.launchpad.net/~lifeless/python-oops-tools/bug-888866/+merge/81933
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/python-oops-tools/bug-888866 into lp:python-oops-tools.

=== modified file 'src/oopstools/NEWS.txt'
--- src/oopstools/NEWS.txt	2011-11-11 03:30:19 +0000
+++ src/oopstools/NEWS.txt	2011-11-11 04:10:27 +0000
@@ -15,6 +15,9 @@
   a bit better.
   (Robert Collins, William Grant, Roman Yepishev, #885416, #884265)
 
+* The req_vars variable in OOPS reports may now be a dict.
+  (Robert Collins, #888866)
+
 0.6.1
 =====
 

=== modified file 'src/oopstools/oops/models.py'
--- src/oopstools/oops/models.py	2011-11-11 03:29:19 +0000
+++ src/oopstools/oops/models.py	2011-11-11 04:10:27 +0000
@@ -305,16 +305,24 @@
         is_local_referrer = False)
     # Grab data needed by the Oops database model from the req_vars.
     req_vars = oops.get('req_vars') or ()
-    # 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):
+    # Some badly written OOPSes are tuples with single item tuples. (field.blob
+    # was seen).
+    # New ones are dicts.
+    def ensure_dict(iterable_or_dict):
+        try:
+            items = iterable_or_dict.items()
+            return iterable_or_dict
+        except AttributeError:
+            result = {}
+            for item in iterable_or_dict:
+                try:
+                    key, value = item
+                except ValueError:
+                    key = item[0]
+                    value = ''
+                result[key] = value
+            return result
+    for key, value in ensure_dict(req_vars).items():
         if isinstance(value, str):
             try:
                 # We can get anything in HTTP headers

=== modified file 'src/oopstools/oops/test/test_dboopsloader.py'
--- src/oopstools/oops/test/test_dboopsloader.py	2011-11-11 03:29:19 +0000
+++ src/oopstools/oops/test/test_dboopsloader.py	2011-11-11 04:10:27 +0000
@@ -177,3 +177,12 @@
         # 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')
+
+    def test_dict_reqvars_bug_888866(self):
+        # If req_vars is a dict, it still processes stuff.
+        report = {
+            'id': 'bug-888866',
+            'req_vars': {'HTTP_USER_AGENT': 'myuseragent'}
+        }
+        oops = parsed_oops_to_model_oops(report, 'bug-888866')
+        self.assertEqual('myuseragent', oops.user_agent)