LGTM with comments.
https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py
File tools/push-to-trunk/common_includes.py (right):
https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py#newcode243
tools/push-to-trunk/common_includes.py:243: retry_on: A callback that
takes the result of the function and returns True
nit: 80col
https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py#newcode246
tools/push-to-trunk/common_includes.py:246: wait_plan: A list of waiting
delays between retries in seconds. The maximum
nit: 80col
https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py#newcode250
tools/push-to-trunk/common_includes.py:250: wait_plan = list(wait_plan
or [])
I don't think the explicit list() conversion is needed.
https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py#newcode251
tools/push-to-trunk/common_includes.py:251: # Sentinel for the first
try.
s/first/last/! This is seriously misleading.
Here's a slight refactoring that doesn't need the sentinel:
wait_plan.reverse()
while True:
try:
result = cb()
except Exception:
got_exception = True
if got_exception or retry_on(result):
if not wait_plan:
raise Exception("Retried too often. Giving up.")
wait_time = wait_plan.pop()
print "Waiting for %f seconds." % wait_time
self._side_effect_handler.Sleep(wait_time)
print "Retrying..."
else:
return result
https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/common_includes.py#newcode286
tools/push-to-trunk/common_includes.py:286: wait_plan = wait_plan or [3,
10, 30]
I'd give the server much more time. Maybe [3, 60, 600].
https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/test_scripts.py
File tools/push-to-trunk/test_scripts.py (right):
https://codereview.chromium.org/91733003/diff/150001/tools/push-to-trunk/test_scripts.py#newcode272
tools/push-to-trunk/test_scripts.py:272: def Wait(seconds):
I think you mean s/Wait/Sleep/.
https://codereview.chromium.org/91733003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
