starlingx
/
distcloud
Code Issues Proposed changes

Update patch set 12 

Patch Set 12:

(15 comments)

Patch-set: 12
CC: Gerrit User 35230 <35230@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
This commit is contained in:
Gerrit User 35230 2024-05-06 21:58:19 +00:00 committed by Gerrit Code Review
parent 3bf40ec074
commit bc2c74725c
1 changed files with 349 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

349
d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68 Normal file
View File

@ -0,0 +1,349 @@
{
"comments": [
{
"unresolved": true,
"key": {
"uuid": "d22aec9d_9b57feed",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 125,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "subcloud_id",
"range": {
"startLine": 125,
"startChar": 30,
"endLine": 125,
"endChar": 41
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "57509b73_07308de5",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 136,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "Suggestion to improve readability:\n\n region_name \u003d {\n 1: FAKE_SITE1_SUBCLOUD1_REGION_NAME,\n 2: FAKE_SITE1_SUBCLOUD2_REGION_NAME\n }\n\n deploy_status \u003d {\n 1: consts.DEPLOY_STATE_SECONDARY,\n 2: consts.DEPLOY_STATE_SECONDARY_FAILED,\n 3: consts.DEPLOY_STATE_SECONDARY,\n }\n\n return {\n \u0027id\u0027: subcloud_id,\n \u0027name\u0027: f\u0027subcloud{subcloud_id}\u0027,\n \u0027region-name\u0027: region_name.get(subcloud_id, \u0027subcloud3\u0027),\n \u0027deploy-status\u0027: deploy_status.get(subcloud_id, consts.DEPLOY_STATE_REHOMING)\n }",
"range": {
"startLine": 127,
"startChar": 0,
"endLine": 136,
"endChar": 9
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "2341a290_ac3772c6",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 363,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "I don\u0027t think we should create a new association here since on line 387 we are getting the association created on the setUp function on line 120 (it\u0027s using the self.association.id on the get call). This means the assertion wont assert against this newly created association.\n\nIt\u0027s better to update the existing association.",
"range": {
"startLine": 359,
"startChar": 7,
"endLine": 363,
"endChar": 63
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "ba89cad3_41a01425",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 464,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "test_get_peer_ks_client_fail",
"range": {
"startLine": 464,
"startChar": 8,
"endLine": 464,
"endChar": 31
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "6acc1947_b5fc876e",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 479,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "Suggestion: since this repeats a lot in the following tests, consider creating a utility method that creates the data_install and the create_subcloud_with_pg_static with default values, then call this method during the following tests passing only the different parameters.",
"range": {
"startLine": 472,
"startChar": 8,
"endLine": 479,
"endChar": 38
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "03158ff2_b754a57f",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 544,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "rehome_data \u003d {}",
"range": {
"startLine": 543,
"startChar": 8,
"endLine": 544,
"endChar": 9
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "dabddf57_4ea9c8bc",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 598,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "This is missing the assertion",
"range": {
"startLine": 591,
"startChar": 0,
"endLine": 598,
"endChar": 39
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "1a1afa63_f57f8ab6",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 643,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "By the test name I was expecting it to test only against the PeerGroupAssociationNotFound exception. It might be better to remove the FakeException and use a mock that returns a fake association so that the function can continue to run after the add_peer_group_association call.",
"range": {
"startLine": 642,
"startChar": 12,
"endLine": 643,
"endChar": 45
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "becbde72_6b82ac1c",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 660,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "Can the self.spm be used here? Same comment applies to the test bellow this one.",
"range": {
"startLine": 660,
"startChar": 8,
"endLine": 660,
"endChar": 11
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "38033283_0aa7a5ce",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 681,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "I think this test is doing the same thing as the previous one.",
"range": {
"startLine": 675,
"startChar": 4,
"endLine": 681,
"endChar": 67
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "63a7f8e8_129ef5dd",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 683,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "typo: priority",
"range": {
"startLine": 683,
"startChar": 38,
"endLine": 683,
"endChar": 47
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "06858a25_24f9475c",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 746,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "Also worth asserting that the function returns the self.peer.id as part of its failed_peer_ids return list.",
"range": {
"startLine": 746,
"startChar": 8,
"endLine": 746,
"endChar": 43
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "909d82f8_5dd561af",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 757,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "Could also assert that the function returns 2 empty iterables when the association is not found.",
"range": {
"startLine": 757,
"startChar": 8,
"endLine": 757,
"endChar": 43
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "1266e9c7_b4caa20c",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 766,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "typo: priority",
"range": {
"startLine": 766,
"startChar": 43,
"endLine": 766,
"endChar": 52
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "ea0e659c_f343ed26",
"filename": "distributedcloud/dcmanager/tests/unit/manager/test_system_peer_manager.py",
"patchSetId": 12
},
"lineNbr": 802,
"author": {
"id": 35230
},
"writtenOn": "2024-05-06T21:58:19Z",
"side": 1,
"message": "Consider asserting that the peer group association was deleted by querying the DB after the delete_peer_group_association call",
"range": {
"startLine": 802,
"startChar": 8,
"endLine": 802,
"endChar": 34
},
"revId": "d1c2a52deab67ba6b1cdeb3b40cc893f2025eb68",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}