Update patch set 10

Patch Set 10: Code-Review-1

(7 comments)

Thanks, I looked into the code a bit more and have more comments/thoughts about the interaction with 'durable'.

Patch-set: 10
Reviewer: Gerrit User 6618 <6618@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1

18d97ea958bdde590bd0e3bcba595d8ca9ce9514

@@ -0,0 +1,106 @@
+{
+  "comments": [
+    {
+      "key": {
+        "uuid": "9a4aa940_8634248a",
+        "filename": "oslo_messaging/_drivers/impl_rabbit.py",
+        "patchSetId": 10
+      },
+      "lineNbr": 153,
+      "author": {
+        "id": 6618
+      },
+      "writtenOn": "2021-10-07T15:00:29Z",
+      "side": 1,
+      "message": "I thought this is disabled (even if set to True) if rabbit_quorum_queue is enabled, based on what it says below at L163: \u0027in other words it will disable HA queues\u0027",
+      "revId": "18d97ea958bdde590bd0e3bcba595d8ca9ce9514",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": true
+    },
+    {
+      "key": {
+        "uuid": "59ecb9f6_c46548c5",
+        "filename": "oslo_messaging/_drivers/impl_rabbit.py",
+        "patchSetId": 10
+      },
+      "lineNbr": 243,
+      "author": {
+        "id": 6618
+      },
+      "writtenOn": "2021-10-07T15:00:29Z",
+      "side": 1,
+      "message": "If I understand, might be useful to s/restart like quorum queues which always durable/restarts. Quorum queues will only be done if durable is true./",
+      "revId": "18d97ea958bdde590bd0e3bcba595d8ca9ce9514",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": true
+    },
+    {
+      "key": {
+        "uuid": "55545c63_8cccd45d",
+        "filename": "oslo_messaging/_drivers/impl_rabbit.py",
+        "patchSetId": 10
+      },
+      "lineNbr": 250,
+      "author": {
+        "id": 6618
+      },
+      "writtenOn": "2021-10-07T15:00:29Z",
+      "side": 1,
+      "message": "(Continued from comment in patch set 8) It seems like in some places in the code, if rabbit_quorum_queue is set, then durable is also set, regardless of the value of the config \u0027amqp_durable_queues\u0027.\n\nIf the Consumer object is something that can be used outside of this library, at the very least, shouldn\u0027t we document that if rabbit_ha_queues is set, that durable needs to be set to true too?\n\nIt would help me wrt reviewing if I understood what the expected behaviour is wrt this new rabbit_ha_queues. Does/should the code always use durable (\u003dTrue) if rabbit_ha_queues is set? If yes, then it should be coded that way and documented as such. If no, then in the cases where durable is False, maybe we need to log something to inform Operators that rabbit_mq_queue was NOT set even though they wanted it set, because durable was False. OR we set it and log that it won\u0027t actually work cuz durable was False.\n\nNote that the config value for amqp_durable_queues is ignored and durable is set to True in some cases -- the user ought to be informed of this. Maybe in the new config help message, in the commit, and in the release note.",
+      "revId": "18d97ea958bdde590bd0e3bcba595d8ca9ce9514",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": true
+    },
+    {
+      "key": {
+        "uuid": "2afade1e_8e012810",
+        "filename": "oslo_messaging/_drivers/impl_rabbit.py",
+        "patchSetId": 10
+      },
+      "lineNbr": 1182,
+      "author": {
+        "id": 6618
+      },
+      "writtenOn": "2021-10-07T15:00:29Z",
+      "side": 1,
+      "message": "Why isn\u0027t this \u003d self.durable? if it is False, then it makes no sense to add L1189.",
+      "revId": "18d97ea958bdde590bd0e3bcba595d8ca9ce9514",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": true
+    },
+    {
+      "key": {
+        "uuid": "4589588d_25e745e5",
+        "filename": "oslo_messaging/_drivers/impl_rabbit.py",
+        "patchSetId": 10
+      },
+      "lineNbr": 1223,
+      "author": {
+        "id": 6618
+      },
+      "writtenOn": "2021-10-07T15:00:29Z",
+      "side": 1,
+      "message": "Why isn\u0027t this \u003d self.durable? if it is False, then it makes no sense to add L1230.",
+      "revId": "18d97ea958bdde590bd0e3bcba595d8ca9ce9514",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": true
+    },
+    {
+      "key": {
+        "uuid": "19bae9c4_49d6f884",
+        "filename": "oslo_messaging/_drivers/impl_rabbit.py",
+        "patchSetId": 10
+      },
+      "lineNbr": 1316,
+      "author": {
+        "id": 6618
+      },
+      "writtenOn": "2021-10-07T15:00:29Z",
+      "side": 1,
+      "message": "i\u0027m not sure -- what does it mean if this is False and rabbit_quorum_queue is True. Will the behaviour be what the user expected?",
+      "revId": "18d97ea958bdde590bd0e3bcba595d8ca9ce9514",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": true
+    }
+  ]
+}

d1cb43cf3fcb972dca3e0b19f8f16a4d04302833

@@ -69,6 +69,24 @@
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
       "unresolved": true
     },
+    {
+      "key": {
+        "uuid": "135bcc57_92bf6cb3",
+        "filename": "oslo_messaging/_drivers/impl_rabbit.py",
+        "patchSetId": 8
+      },
+      "lineNbr": 276,
+      "author": {
+        "id": 6618
+      },
+      "writtenOn": "2021-10-07T15:00:29Z",
+      "side": 1,
+      "message": "Done",
+      "parentUuid": "e930031e_4a2dce3e",
+      "revId": "d1cb43cf3fcb972dca3e0b19f8f16a4d04302833",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
     {
       "key": {
         "uuid": "767570c1_3452bce2",