Update patch set 12

Patch Set 12: Code-Review-1

(9 comments)

Patch-set: 12
Reviewer: Gerrit User 9535 <9535@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1
Attention: {"person_ident":"Gerrit User 9535 \u003c9535@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"Gorka Eguileor replied on the change"}
This commit is contained in:
Gerrit User 9535 2022-05-24 12:36:42 +00:00 committed by Gerrit Code Review
parent 1dd5f3a65c
commit 6e28c9598e
2 changed files with 194 additions and 0 deletions

View File

@ -276,6 +276,24 @@
"revId": "a49ab19d54d5661f544d931e649655193cbf6044",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "ed737f13_dae12ae4",
"filename": "os_brick/initiator/connectors/nvmeof.py",
"patchSetId": 7
},
"lineNbr": 123,
"author": {
"id": 9535
},
"writtenOn": "2022-05-24T12:36:42Z",
"side": 1,
"message": "In my opinion we should remove L121-123 for now, so method get_connector_properties doesn\u0027t log the warning.\n\nThat way deployments that only use iSCSI or FC with multipathing won\u0027t be seeing this warning on every single attach.",
"parentUuid": "4f8fdf1d_8db0e4e3",
"revId": "a49ab19d54d5661f544d931e649655193cbf6044",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {

View File

@ -0,0 +1,176 @@
{
"comments": [
{
"unresolved": false,
"key": {
"uuid": "46967367_95062425",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 12
},
"lineNbr": 0,
"author": {
"id": 9535
},
"writtenOn": "2022-05-24T12:36:42Z",
"side": 1,
"message": "There are some nits that can be left for future improvements, the downvote is because there is a case where the code could be returning that it is not connected to a portal when it is (L577 of nvmeof.py file).\n\nI think I missed it in previous reviews.",
"revId": "ad2bc618fae467af1655e5e73765a4e859bb3bee",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "f9957a75_0f022f01",
"filename": "os_brick/initiator/connectors/nvmeof.py",
"patchSetId": 12
},
"lineNbr": 522,
"author": {
"id": 9535
},
"writtenOn": "2022-05-24T12:36:42Z",
"side": 1,
"message": "nit: This could go inside the else clause on L528",
"range": {
"startLine": 521,
"startChar": 0,
"endLine": 522,
"endChar": 23
},
"revId": "ad2bc618fae467af1655e5e73765a4e859bb3bee",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "1a442053_8db48600",
"filename": "os_brick/initiator/connectors/nvmeof.py",
"patchSetId": 12
},
"lineNbr": 576,
"author": {
"id": 9535
},
"writtenOn": "2022-05-24T12:36:42Z",
"side": 1,
"message": "nit: No need for else, because the break jumps to L578",
"range": {
"startLine": 576,
"startChar": 16,
"endLine": 576,
"endChar": 20
},
"revId": "ad2bc618fae467af1655e5e73765a4e859bb3bee",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "4d3253c8_04693698",
"filename": "os_brick/initiator/connectors/nvmeof.py",
"patchSetId": 12
},
"lineNbr": 577,
"author": {
"id": 9535
},
"writtenOn": "2022-05-24T12:36:42Z",
"side": 1,
"message": "-1: This should mark that the connection exists:\n\n any_connect \u003d True",
"revId": "ad2bc618fae467af1655e5e73765a4e859bb3bee",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "38875126_64d2fc39",
"filename": "os_brick/initiator/connectors/nvmeof.py",
"patchSetId": 12
},
"lineNbr": 599,
"author": {
"id": 9535
},
"writtenOn": "2022-05-24T12:36:42Z",
"side": 1,
"message": "nit:\n\n address \u003d f\"traddr\u003d{portal_address},trsvcid\u003d{portal_port}\"",
"range": {
"startLine": 596,
"startChar": 0,
"endLine": 599,
"endChar": 73
},
"revId": "ad2bc618fae467af1655e5e73765a4e859bb3bee",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "468ff1fe_62b49ef2",
"filename": "os_brick/initiator/connectors/nvmeof.py",
"patchSetId": 12
},
"lineNbr": 602,
"author": {
"id": 9535
},
"writtenOn": "2022-05-24T12:36:42Z",
"side": 1,
"message": "nit:\n\n return address in nvme_ctrls",
"range": {
"startLine": 600,
"startChar": 0,
"endLine": 602,
"endChar": 20
},
"revId": "ad2bc618fae467af1655e5e73765a4e859bb3bee",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "f620b924_29098293",
"filename": "os_brick/initiator/connectors/nvmeof.py",
"patchSetId": 12
},
"lineNbr": 632,
"author": {
"id": 9535
},
"writtenOn": "2022-05-24T12:36:42Z",
"side": 1,
"message": "nit: Address is readable by everyone on the system, so we don\u0027t need privileged access to read it and we can read it in Python instead of running a subcommand.",
"range": {
"startLine": 629,
"startChar": 0,
"endLine": 632,
"endChar": 72
},
"revId": "ad2bc618fae467af1655e5e73765a4e859bb3bee",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "bd86bc16_4448c87d",
"filename": "os_brick/initiator/connectors/nvmeof.py",
"patchSetId": 12
},
"lineNbr": 633,
"author": {
"id": 9535
},
"writtenOn": "2022-05-24T12:36:42Z",
"side": 1,
"message": "nit:\n\n ctrl_name \u003d os.path.basename(ctrl)",
"range": {
"startLine": 633,
"startChar": 40,
"endLine": 633,
"endChar": 66
},
"revId": "ad2bc618fae467af1655e5e73765a4e859bb3bee",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}