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

appengine/findit/handlers/handlers_util.py

 # Copyright 2016 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 from collections import defaultdict
 import copy
 
 from handlers import result_status
 from model import analysis_status
 from model.wf_analysis import WfAnalysis
@@ skipping 212 matching lines @@
           try_job_info['try_job_build_number'] = (
               _GetTryJobBuildNumber(try_job_result['url']))
 
         if (try_job_result.get('culprit') and
             try_job_result['culprit'].get(ref_name)):
           # Saves try job culprits information.
 
           # 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
           else:
             # Saves the first culprit in current try_job_info.
             # Saves all the other culprits later.
             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']
 
           for n in xrange(index_start, len(list_of_culprits)):
             # Appends the rest of test groups to step_try_jobs['try_jobs'].
             iterate_culprit = list_of_culprits[n]
             tmp_try_job_info = copy.deepcopy(try_job_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)
 
     if additional_tests_culprit_info:
       step_try_jobs['try_jobs'].extend(additional_tests_culprit_info)
 
 
 def _UpdateTryJobInfoBasedOnSwarming(step_tasks_info, try_jobs):
   """
@@ skipping 138 matching lines @@
 
       culprits_info[step_name] = {
           'try_jobs': [
               {
                   'status': result_status.NO_FAILURE_RESULT_MAP,
                   'tests': tests
               }
           ]
       }
   return culprits_info