Browse Source

Apply the gerrit REST timeout to only 'GetChanges' calls

https://crrev.com/c/4420526 put a 10s timeout on all calls made via
this script.

But it appears there are a variety of endpoints that take more than
10s, the latest being "SubmitChange", eg:
https://ci.chromium.org/ui/p/chromeos/builders/release/release-R113-15393.B-orchestrator/25/overview

So instead of applying the timeout to everything, and then opting
certain long-running calls out of it, this just opts-in the
"GetChanges" call to the timeout. This should cover the failure mode
that was originally solved for chrome's bots. And also bump the
timeout to 30s since we don't trust all get-changes queries to
reliably finish in under 10s.

This also bumps the step level timeout for the query in tryserver
recipe_mod to 8 min since there could be ~5min worth of sleeping +
150s worth of waiting+timing-out. So worst case the step will now
take 8min to fully exhaust all timeouts/sleeps.

Recipe-Nontrivial-Roll: build
Recipe-Nontrivial-Roll: build_limited
Recipe-Nontrivial-Roll: chromiumos
Recipe-Nontrivial-Roll: infra
Bug: b/278083716
Change-Id: Ib366e004e0bb07297ba732590d488cae779e38ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4426524
Reviewed-by: Joanna Wang <jojwang@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Ben Pastene 2 năm trước cách đây
mục cha
commit
97dadd025f
18 tập tin đã thay đổi với 32 bổ sung32 xóa
  1. 3 3
      gerrit_util.py
  2. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json
  3. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail.json
  4. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch.json
  5. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch_download.json
  6. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_angle.json
  7. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_branch_heads.json
  8. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_feature_branch.json
  9. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_v8_feature_branch.json
  10. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json
  11. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/tryjob_v8.json
  12. 1 1
      recipes/recipe_modules/bot_update/examples/full.expected/tryjob_v8_head_by_default.json
  13. 2 2
      recipes/recipe_modules/tryserver/api.py
  14. 4 4
      recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json
  15. 4 4
      recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json
  16. 1 1
      recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json
  17. 1 1
      recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json
  18. 6 6
      tests/gerrit_util_test.py

+ 3 - 3
gerrit_util.py

@@ -380,7 +380,7 @@ def CreateHttpConn(host,
                    reqtype='GET',
                    headers=None,
                    body=None,
-                   timeout=10.0):
+                   timeout=None):
   """Opens an HTTPS connection to a Gerrit service, and sends a request."""
   headers = headers or {}
   bare_host = host.partition(':')[0]
@@ -556,7 +556,7 @@ def QueryChanges(host, params, first_param=None, limit=None, o_params=None,
     path = '%s&n=%d' % (path, limit)
   if o_params:
     path = '%s&%s' % (path, '&'.join(['o=%s' % p for p in o_params]))
-  return ReadHttpJsonResponse(CreateHttpConn(host, path))
+  return ReadHttpJsonResponse(CreateHttpConn(host, path, timeout=30.0))
 
 
 def GenerateAllChanges(host, params, first_param=None, limit=500,
@@ -1062,7 +1062,7 @@ def CreateChange(host, project, branch='main', subject='', params=()):
     if not body[key]:
       raise GerritError(200, '%s is required' % key.title())
 
-  conn = CreateHttpConn(host, path, reqtype='POST', body=body, timeout=None)
+  conn = CreateHttpConn(host, path, reqtype='POST', body=body)
   return ReadHttpJsonResponse(conn, accept_statuses=[201])
 
 

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/deprecated_got_revision_mapping.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/tryjob_fail_patch_download.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_angle.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_branch_heads.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_feature_branch.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_v8_feature_branch.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/tryjob_v8.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/bot_update/examples/full.expected/tryjob_v8_head_by_default.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 2 - 2
recipes/recipe_modules/tryserver/api.py

@@ -140,7 +140,7 @@ class TryserverApi(recipe_api.RecipeApi):
         o_params=['ALL_REVISIONS', 'DOWNLOAD_COMMANDS'],
         limit=1,
         name='fetch current CL info',
-        timeout=360,
+        timeout=480,
         step_test_data=lambda: self.m.json.test_api.output(mock_res))[0]
 
     self._gerrit_change_target_ref = res['branch']
@@ -348,7 +348,7 @@ class TryserverApi(recipe_api.RecipeApi):
         'https://%s' % self.gerrit_change.host,
         self.gerrit_change_number,
         self.gerrit_patchset_number,
-        timeout=360)
+        timeout=480)
 
   def _get_footers(self, patch_text=None):
     if patch_text is not None:

+ 4 - 4
recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",
@@ -90,7 +90,7 @@
       }
     },
     "name": "gerrit changes",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",
@@ -179,7 +179,7 @@
       }
     },
     "name": "gerrit changes (2)",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",
@@ -330,7 +330,7 @@
       }
     },
     "name": "gerrit fetch current CL info (2)",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 4 - 4
recipes/recipe_modules/tryserver/examples/full.expected/with_gerrit_patch_and_target_ref.json

@@ -35,7 +35,7 @@
       }
     },
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",
@@ -90,7 +90,7 @@
       }
     },
     "name": "gerrit changes",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",
@@ -179,7 +179,7 @@
       }
     },
     "name": "gerrit changes (2)",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",
@@ -330,7 +330,7 @@
       }
     },
     "name": "gerrit fetch current CL info (2)",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch.json

@@ -23,7 +23,7 @@
     },
     "infra_step": true,
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 1 - 1
recipes/recipe_modules/tryserver/examples/full.expected/with_wrong_patch_new.json

@@ -23,7 +23,7 @@
     },
     "infra_step": true,
     "name": "gerrit fetch current CL info",
-    "timeout": 360,
+    "timeout": 480,
     "~followup_annotations": [
       "@@@STEP_LOG_LINE@json.output@[@@@",
       "@@@STEP_LOG_LINE@json.output@  {@@@",

+ 6 - 6
tests/gerrit_util_test.py

@@ -439,12 +439,12 @@ class GerritUtilTest(unittest.TestCase):
         'host', [('key', 'val'), ('foo', 'bar baz')], 'first param', limit=500,
         o_params=['PARAM_A', 'PARAM_B'], start='start')
     mockCreateHttpConn.assert_called_once_with(
-        'host',
-        ('changes/?q=first%20param+key:val+foo:bar+baz'
-         '&start=start'
-         '&n=500'
-         '&o=PARAM_A'
-         '&o=PARAM_B'))
+        'host', ('changes/?q=first%20param+key:val+foo:bar+baz'
+                 '&start=start'
+                 '&n=500'
+                 '&o=PARAM_A'
+                 '&o=PARAM_B'),
+        timeout=30.0)
 
   def testQueryChanges_NoParams(self):
     self.assertRaises(RuntimeError, gerrit_util.QueryChanges, 'host', [])