[Findit] Fix Key_error: 'review_url' in handlers_util.py

appengine/findit/handlers/handlers_util.py

Index: appengine/findit/handlers/handlers_util.py
diff --git a/appengine/findit/handlers/handlers_util.py b/appengine/findit/handlers/handlers_util.py
index e8194774cca0127a77033ce10e217c5a9187c28c..072cfe260a1f605256eaffdc99d800ab33cb9963 100644
--- a/appengine/findit/handlers/handlers_util.py
+++ b/appengine/findit/handlers/handlers_util.py
@@ -230,24 +230,24 @@ def _GetCulpritInfoForTryJobResultForTest(try_job_key, culprits_info):
           # Uses culprits to group tests.
           culprit_tests_map = _OrganizeTryJobResultByCulprits(
               try_job_result['culprit'][ref_name])
-          unrgouped_tests = try_job_info['tests']
+          ungrouped_tests = try_job_info['tests']
           list_of_culprits = []
           for culprit_info in culprit_tests_map.values():
             failed_tests = culprit_info['failed_tests']
             list_of_culprits.append(culprit_info)
             # Gets tests that haven't been grouped.
-            unrgouped_tests = list(
-                set(unrgouped_tests) ^ set(failed_tests))
-            if not unrgouped_tests:
+            ungrouped_tests = list(
+                set(ungrouped_tests) ^ set(failed_tests))
+            if not ungrouped_tests:
               # All tests have been grouped.
               break
 
           index_start = 1
-          if unrgouped_tests:
+          if ungrouped_tests:
             # There are tests don't have try job culprits.
             # Group these tests together.
             # Save them in current try_job_info.
-            try_job_info['tests'] = unrgouped_tests
+            try_job_info['tests'] = ungrouped_tests
             try_job_info['culprit'] = {}
             # Saves all the tests that have culprits later.
             index_start = 0
@@ -257,7 +257,8 @@ def _GetCulpritInfoForTryJobResultForTest(try_job_key, culprits_info):
             try_job_info['culprit'] = {
                 'revision': list_of_culprits[0]['revision'],
                 'commit_position': list_of_culprits[0]['commit_position'],
-                'review_url': list_of_culprits[0]['review_url']
+                'review_url': list_of_culprits[0].get(

[lijeffrey, 2016/05/03 23:39:35]

so is it possible for there to be revision and commit position but not review
url?

[chanli, 2016/05/03 23:45:20]

Just in case we change the key name once again. Although theoretically it
shouldn't happen. 

+                    'url',list_of_culprits[0].get('review_url', None))

[stgao, 2016/05/03 23:38:28]

style nit: "'url', list..."

Same below.

[lijeffrey, 2016/05/03 23:39:35]

nit: space after ,

[chanli, 2016/05/03 23:45:20]

Done.

[chanli, 2016/05/03 23:45:20]

Done.

             }
             try_job_info['tests'] = list_of_culprits[0]['failed_tests']
 
@@ -268,7 +269,8 @@ def _GetCulpritInfoForTryJobResultForTest(try_job_key, culprits_info):
             tmp_try_job_info['culprit'] = {
                 'revision': iterate_culprit['revision'],
                 'commit_position': iterate_culprit['commit_position'],
-                'review_url': iterate_culprit['review_url']
+                'review_url': iterate_culprit.get(
+                    'url',iterate_culprit.get('review_url', None))

[lijeffrey, 2016/05/03 23:39:35]

nit: space after ,

[chanli, 2016/05/03 23:45:20]

Done.

             }
             tmp_try_job_info['tests'] = iterate_culprit['failed_tests']
             additional_tests_culprit_info.append(tmp_try_job_info)