Update patch set 11

Patch Set 11: Code-Review-1

(7 comments)

Thanks, I've got some minor comments.

Patch-set: 11
Reviewer: Gerrit User 6618 <6618@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1
This commit is contained in:
Gerrit User 6618 2021-10-07 16:03:37 +00:00 committed by Gerrit Code Review
parent be8c313d20
commit 245193d317
2 changed files with 124 additions and 0 deletions

View File

@ -191,6 +191,24 @@
"revId": "18d97ea958bdde590bd0e3bcba595d8ca9ce9514",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "e2a5abca_fb07019a",
"filename": "oslo_messaging/_drivers/impl_rabbit.py",
"patchSetId": 10
},
"lineNbr": 1316,
"author": {
"id": 6618
},
"writtenOn": "2021-10-07T16:03:37Z",
"side": 1,
"message": "ah, thx, I didn\u0027t look to see where/what exchange came from :)",
"parentUuid": "cb1b5fbb_d76347db",
"revId": "18d97ea958bdde590bd0e3bcba595d8ca9ce9514",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
}
]
}

View File

@ -0,0 +1,106 @@
{
"comments": [
{
"key": {
"uuid": "df5b8fe9_88438fca",
"filename": "oslo_messaging/_drivers/amqp.py",
"patchSetId": 11
},
"lineNbr": 35,
"author": {
"id": 6618
},
"writtenOn": "2021-10-07T16:03:37Z",
"side": 1,
"message": "maybe \u0027If rabbit_quorum_queue is enabled, queues will be durable and this value will be ignored\u0027.",
"revId": "9e2ae43834d8d47ca7d4d9eaafe6a64049165b31",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "8a226747_35814573",
"filename": "oslo_messaging/_drivers/impl_rabbit.py",
"patchSetId": 11
},
"lineNbr": 152,
"author": {
"id": 6618
},
"writtenOn": "2021-10-07T16:03:37Z",
"side": 1,
"message": "nit s/this will be/This will be/ \n\nand I think it is missing a period after \u0027enabled\u0027.",
"revId": "9e2ae43834d8d47ca7d4d9eaafe6a64049165b31",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "e7ba2eab_f644dee0",
"filename": "oslo_messaging/_drivers/impl_rabbit.py",
"patchSetId": 11
},
"lineNbr": 162,
"author": {
"id": 6618
},
"writtenOn": "2021-10-07T16:03:37Z",
"side": 1,
"message": "I think we should add something here, about \u0027If this is enabled, the queue will be durable and ``amqp_durable_queues`` option is ignored.",
"revId": "9e2ae43834d8d47ca7d4d9eaafe6a64049165b31",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "c9cf7edc_2bb48208",
"filename": "oslo_messaging/_drivers/impl_rabbit.py",
"patchSetId": 11
},
"lineNbr": 250,
"author": {
"id": 6618
},
"writtenOn": "2021-10-07T16:03:37Z",
"side": 1,
"message": "Based on my understanding, it looks like the code has been changed so that if rabbit_quorum_queue config option is enabled (True), then durable will be True. If that is the case, we don\u0027t need to pass \u0027durable\u0027 to this method.\n\nI see these options:\n- don\u0027t pass \u0027durable\u0027 and assume the code is correct\n- pass \u0027durable\u0027 and if \u0027durable\u0027 is False, raise exception (I\u0027m assuming we think it is incorrect for durable to be False and rabbit_quorum_queue is True)\n\nAs an aside, I thought I saw in passing when I was trying to understand this, that for rabbit_ha_queues, durable needed to be True as well so I suspect that the existing code is allowing for durable\u003dFalse with rabbit_ha_queues, and it is up to the operator to know this :-(",
"revId": "9e2ae43834d8d47ca7d4d9eaafe6a64049165b31",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "ab1cfc0c_a9d54f45",
"filename": "oslo_messaging/_drivers/impl_rabbit.py",
"patchSetId": 11
},
"lineNbr": 280,
"author": {
"id": 6618
},
"writtenOn": "2021-10-07T16:03:37Z",
"side": 1,
"message": "i notice below, that Consumer() is called with rabbit_quorum_queue\u003dFalse. Perhaps this should be s/None/False/ ?",
"revId": "9e2ae43834d8d47ca7d4d9eaafe6a64049165b31",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "aba3478d_65da3748",
"filename": "releasenotes/notes/adding_support_for_quorum_queues-3101d055b492289e.yaml",
"patchSetId": 11
},
"lineNbr": 10,
"author": {
"id": 6618
},
"writtenOn": "2021-10-07T16:03:37Z",
"side": 1,
"message": "i don\u0027t know if it is worth adding something here about the queues being durable and amqp_durable_queues config option being ignored or overridden or whatever the wording is.",
"revId": "9e2ae43834d8d47ca7d4d9eaafe6a64049165b31",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
}
]
}