121 lines
4.6 KiB
Plaintext
121 lines
4.6 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "d9f460df_245694fc",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 4523
|
|
},
|
|
"writtenOn": "2022-04-05T21:56:16Z",
|
|
"side": 1,
|
|
"message": "Since we are changing code that affects extend and disconnect volume... is there a risk of anything happening when nova-compute is upgraded to use this version of os-brick, without detaching volumes that are currently in use?\n",
|
|
"revId": "c76d13ba7f7335277240e45e9a0007aad10dac03",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "02e76b6d_5caf8408",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 9535
|
|
},
|
|
"writtenOn": "2022-04-08T11:36:15Z",
|
|
"side": 1,
|
|
"message": "I took that into consideration when writing the code, so besides the risk that I inadvertently introduced a bug I don\u0027t think it\u0027d be a problem.\n\nThe reason is that the `connect_volume_undo_prepare_result` method uses the `_device_path_from_symlink` method to replace the symlink with the one that the connector originally returned, and that method explicitly looks for the custom symlink format (which is impossible to have been used by any of the existing connections) and if it doesn\u0027t match then it leaves the path as it was.\n\nThat way existing encrypted connections will not have their paths modified. The only difference will be that they get a copy of the dictionary instead of the original one.\n\nIn any case, your concern is so important that I\u0027m going to harden it a bit more so that in those cases **nothing** changes.",
|
|
"parentUuid": "d9f460df_245694fc",
|
|
"revId": "c76d13ba7f7335277240e45e9a0007aad10dac03",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "e552f98e_683163b4",
|
|
"filename": "os_brick/utils.py",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 266,
|
|
"author": {
|
|
"id": 4523
|
|
},
|
|
"writtenOn": "2022-04-05T13:29:52Z",
|
|
"side": 1,
|
|
"message": "There is probably some value in adding type annotations to these to prevent misuse -- i.e.\n def connect_volume_prepare_result(\n func: Callable[[Any, dict], dict]) -\u003e Callable[[Any, dict], dict]:\n\nor so. (We may eventually have a stronger type for connection_properties than just \"dict\", but this is a good start.)",
|
|
"revId": "c76d13ba7f7335277240e45e9a0007aad10dac03",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "df4dce25_afe73965",
|
|
"filename": "os_brick/utils.py",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 266,
|
|
"author": {
|
|
"id": 9535
|
|
},
|
|
"writtenOn": "2022-04-08T11:36:15Z",
|
|
"side": 1,
|
|
"message": "Definitely a good idea.",
|
|
"parentUuid": "e552f98e_683163b4",
|
|
"revId": "c76d13ba7f7335277240e45e9a0007aad10dac03",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "21b77711_c7faae80",
|
|
"filename": "os_brick/utils.py",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 339,
|
|
"author": {
|
|
"id": 4523
|
|
},
|
|
"writtenOn": "2022-04-05T21:56:16Z",
|
|
"side": 1,
|
|
"message": "Not sure what this comment means -- it\u0027s used at line 340?",
|
|
"range": {
|
|
"startLine": 339,
|
|
"startChar": 4,
|
|
"endLine": 339,
|
|
"endChar": 31
|
|
},
|
|
"revId": "c76d13ba7f7335277240e45e9a0007aad10dac03",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "6a20f0c6_c6ed566b",
|
|
"filename": "os_brick/utils.py",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 339,
|
|
"author": {
|
|
"id": 9535
|
|
},
|
|
"writtenOn": "2022-04-08T11:36:15Z",
|
|
"side": 1,
|
|
"message": "Thanks for catching this one.\n\nIt\u0027s a leftover comment from when I thought this was breaking the introspection, but it turns out that it wasn\u0027t this, but the synchronize partial function, so I added warning from L327-328.\n\nWill remove it.",
|
|
"parentUuid": "21b77711_c7faae80",
|
|
"range": {
|
|
"startLine": 339,
|
|
"startChar": 4,
|
|
"endLine": 339,
|
|
"endChar": 31
|
|
},
|
|
"revId": "c76d13ba7f7335277240e45e9a0007aad10dac03",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
}
|
|
]
|
|
} |