Merge lp:~cprov/autopkgtest-result-checker/retry-review into lp:autopkgtest-result-checker/ubuntu-proposed-migration

[Celso Providelo]

Stop retrying exit_code 20 (let them fail fast, as recommended by pitti).

Stop retrying exit_code 20 (let them fail fast, as recommended by pitti).

Right branch this time ...

[Martin Pitt (pitti)]

I'm a bit confused by this -- instead of starting to count from 1, shouldn't this lower max_retries from 4 to 3? FWIW, if a test does not work after 3 retries it's seriously broken, so we don't want to hide that by always having to run it 4 times. So failing in that case is okay. Unless of course it's failure of nova or so -- but this would be retried in uci-nova, not the whole adt-run call.

[Thomi Richards (thomir-deactivatedaccount)]

retry_count is set to 3, but since this component sees code after it's already run once, 3 retries == 4 runs.

I do think that we should fix this by lowering 'retry_count' in the config file though - having a count that starts at '1' is just odd IMO.

I'm fine with the other changes though.

review: Approve

[lp:~cprov/autopkgtest-result-checker/retry-review]

16. By Celso Providelo on 2015-05-29

Reverting the retry counter change, we opted for a configuration change.

17. By Celso Providelo on 2015-05-29

Using constants.MAX_RETRIES instead of configuration [adt].max_retries. Also setting it to 2, so a job will be processed at most 3 times.

[Thomi Richards (thomir-deactivatedaccount)]

suhweeeeeet!

review: Approve

=== modified file 'adt_result_checker/__init__.py'
--- adt_result_checker/__init__.py	2015-04-15 19:22:18 +0000
+++ adt_result_checker/__init__.py	2015-05-29 03:27:54 +0000
@@ -37,6 +37,7 @@
 from adt_result_checker.constants import (
     API_VERSION,
     logging_extra,
+    MAX_RETRIES,
 )
 
 
@@ -160,9 +161,8 @@
         """
         extra = message.payload.copy()
         extra.update(logging_extra)
-        max_retries = int(self.config['adt']['max_retries'])
         current_retries = int(message.payload.get('retry_count', 0))
-        if current_retries < max_retries:
+        if current_retries < MAX_RETRIES:
             payload_copy = message.payload.copy()
             logger.info("Queueing for retry.", extra=extra)
             payload_copy['retry_count'] = current_retries + 1
@@ -232,10 +232,10 @@
         100 Generic error happened.
 
     This function determines which exit codes we should retry on. Currently
-    we will retry on: 16, 20, 100
+    we will retry on: 16, 100
 
     """
-    return exit_code in (16, 20, 100)
+    return exit_code in (16, 100)
 
 
 def configure_logging(config):

=== modified file 'adt_result_checker/constants.py'
--- adt_result_checker/constants.py	2015-04-15 19:05:16 +0000
+++ adt_result_checker/constants.py	2015-05-29 03:27:54 +0000
@@ -22,6 +22,8 @@
 
 API_VERSION = "v1"
 
+MAX_RETRIES = 2
+
 SOLUTION_NAME = "proposed-migration"
 
 SERVICE_NAME = "adt-result-checker"