Update patch set 1

Patch Set 1: Workflow-1

(3 comments)

Patch-set: 1
Reviewer: Gerrit User 21273 <21273@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Workflow=-1

ce9131cd1161866dfb398a30a7e053e195828a52

@@ -17,6 +17,24 @@
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
       "unresolved": false
     },
+    {
+      "key": {
+        "uuid": "fa6399be_c93a2834",
+        "filename": "kuryr_kubernetes/handlers/retry.py",
+        "patchSetId": 1
+      },
+      "lineNbr": 62,
+      "author": {
+        "id": 21273
+      },
+      "writtenOn": "2016-10-19T23:51:54Z",
+      "side": 1,
+      "message": "agreed",
+      "parentUuid": "fa6399be_d4448d1c",
+      "revId": "ce9131cd1161866dfb398a30a7e053e195828a52",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
     {
       "key": {
         "uuid": "fa6399be_f4d191fc",
@@ -40,6 +58,30 @@
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
       "unresolved": false
     },
+    {
+      "key": {
+        "uuid": "fa6399be_8938303f",
+        "filename": "kuryr_kubernetes/handlers/retry.py",
+        "patchSetId": 1
+      },
+      "lineNbr": 66,
+      "author": {
+        "id": 21273
+      },
+      "writtenOn": "2016-10-19T23:51:54Z",
+      "side": 1,
+      "message": "Don\u0027t mind getting rid of \u0027not\u0027 part. Though \u0027should_give_up\u0027 does not sound good, but can make it \u0027should_(re)raise\u0027.\n\nOr we could keep \u0027not\u0027 and rename the method into \u0027_sleep\u0027 and make it return the \u0027interval\u0027 (\u00270\u0027 on timeout). IMO that makes the most sense as \u0027should_...\u0027 sounds like a \u0027check\u0027-type function without side-effects while it actually does \u0027sleep\u0027.\n\nThat \u0027LOG.debug\u0027 duplicates the ones in \u0027_should_retry\u0027 and those two have different meaning, so I\u0027d like to keep them separate instead of replacing them with \u0027LOG.debug\u0027 in \u0027__call__\u0027. We could probably use \u0027pass\u0027 in place of \u0027LOG.debug\u0027 here, but I can\u0027t say I like it any better. Also I prefer to avoid wrapping \u0027with\u0027 if possible.",
+      "parentUuid": "fa6399be_f4d191fc",
+      "range": {
+        "startLine": 64,
+        "startChar": 0,
+        "endLine": 66,
+        "endChar": 65
+      },
+      "revId": "ce9131cd1161866dfb398a30a7e053e195828a52",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
     {
       "key": {
         "uuid": "fa6399be_74602136",
@@ -62,6 +104,30 @@
       "revId": "ce9131cd1161866dfb398a30a7e053e195828a52",
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
       "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "fa6399be_69a59c02",
+        "filename": "kuryr_kubernetes/handlers/retry.py",
+        "patchSetId": 1
+      },
+      "lineNbr": 85,
+      "author": {
+        "id": 21273
+      },
+      "writtenOn": "2016-10-19T23:51:54Z",
+      "side": 1,
+      "message": "I used min/max initially, but it shows 100% coverage even when some cases are not actually tested. And I also do find min/max less readable.",
+      "parentUuid": "fa6399be_74602136",
+      "range": {
+        "startLine": 79,
+        "startChar": 0,
+        "endLine": 85,
+        "endChar": 37
+      },
+      "revId": "ce9131cd1161866dfb398a30a7e053e195828a52",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
     }
   ]
 }