Update patch set 11

Patch Set 11: Code-Review-1

(10 comments)

Patch-set: 11
Reviewer: Gerrit User 28609 <28609@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1, af40e82994d2255c420480588b0e32a285a8db3f
Attention: {"person_ident":"Gerrit User 31664 \u003c31664@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_28609\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 30674 \u003c30674@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_28609\u003e replied on the change"}
This commit is contained in:
Gerrit User 28609 2023-10-25 16:16:08 +00:00 committed by Gerrit Code Review
parent 519eb5ff4a
commit af235bca32
2 changed files with 217 additions and 0 deletions

View File

@ -104,6 +104,30 @@
"revId": "4664bd127ff0051cc22feb7ba54380e47a700da5",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "0724a043_da9fcb28",
"filename": "octavia_tempest_plugin/tests/scenario/v2/test_backup_members.py",
"patchSetId": 6
},
"lineNbr": 49,
"author": {
"id": 28609
},
"writtenOn": "2023-10-25T16:16:08Z",
"side": 1,
"message": "I think that the backup members feature should not be affected by IP version used (it\u0027s transparent) and it should work exactly the same for both.\nSupporting both: IPV4 and IPV6 setups, makes sense to me and if it doesn\u0027t require drastic code changes I would recommend adding this option.\nBTW - there are some lines in the code like L122 or L129 that logs and exports some IPV6 values, but the IPV6 is not in the game as of now.",
"parentUuid": "156cc2eb_4994a3e6",
"range": {
"startLine": 49,
"startChar": 8,
"endLine": 49,
"endChar": 22
},
"revId": "4664bd127ff0051cc22feb7ba54380e47a700da5",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {

View File

@ -0,0 +1,193 @@
{
"comments": [
{
"unresolved": false,
"key": {
"uuid": "18fb6600_c9c6b343",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 11
},
"lineNbr": 0,
"author": {
"id": 28609
},
"writtenOn": "2023-10-25T16:16:08Z",
"side": 1,
"message": "Hi Lukas!\nThere is a need to work on this patch a bit more and to fix some stuff.\nNote: I didn\u0027t review the tests part yet, will do so soon.\nThanks!",
"revId": "ca24bc3e8d170ab9148a002dde8bdd1e3754249e",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "fb6ad159_ed8862d7",
"filename": "octavia_tempest_plugin/tests/scenario/v2/test_backup_members.py",
"patchSetId": 11
},
"lineNbr": 186,
"author": {
"id": 28609
},
"writtenOn": "2023-10-25T16:16:08Z",
"side": 1,
"message": "We probably need a cleanup here, I mean:\ncls.addClassResourceCleanup(cls.mem_member_client.cleanup_member,....\nRelevant for all the rest:1,2,3 and 4",
"revId": "ca24bc3e8d170ab9148a002dde8bdd1e3754249e",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "c17f2897_a003094b",
"filename": "octavia_tempest_plugin/tests/scenario/v2/test_backup_members.py",
"patchSetId": 11
},
"lineNbr": 274,
"author": {
"id": 28609
},
"writtenOn": "2023-10-25T16:16:08Z",
"side": 1,
"message": "Missing cleanup for \"hm\" created?",
"revId": "ca24bc3e8d170ab9148a002dde8bdd1e3754249e",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "066f3d56_102bd919",
"filename": "octavia_tempest_plugin/tests/scenario/v2/test_backup_members.py",
"patchSetId": 11
},
"lineNbr": 275,
"author": {
"id": 28609
},
"writtenOn": "2023-10-25T16:16:08Z",
"side": 1,
"message": "There is a concept to name helper function like this to start with underscore, it means: _check_member_status",
"range": {
"startLine": 275,
"startChar": 8,
"endLine": 275,
"endChar": 27
},
"revId": "ca24bc3e8d170ab9148a002dde8bdd1e3754249e",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "4c038827_25c3ff0b",
"filename": "octavia_tempest_plugin/tests/scenario/v2/test_backup_members.py",
"patchSetId": 11
},
"lineNbr": 284,
"author": {
"id": 28609
},
"writtenOn": "2023-10-25T16:16:08Z",
"side": 1,
"message": "Same as in L274\n+ \nIt\u0027s about getting HTTP response from member, therefore renaming to \"_get_active_members_response\" will be more accurate I think.",
"range": {
"startLine": 284,
"startChar": 8,
"endLine": 284,
"endChar": 26
},
"revId": "ca24bc3e8d170ab9148a002dde8bdd1e3754249e",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "fd1ae090_13e3c80f",
"filename": "octavia_tempest_plugin/tests/scenario/v2/test_backup_members.py",
"patchSetId": 11
},
"lineNbr": 286,
"author": {
"id": 28609
},
"writtenOn": "2023-10-25T16:16:08Z",
"side": 1,
"message": "I think that we need to include a port to create this URL here, to make it work always. Means that the URL needs to have a port taken from listener_kwargs L82.\nFor example, if someone will change listener port from 80 to 85 this L85 this URL won\u0027t be valid anymore as it assumes that the default 80 is used.",
"range": {
"startLine": 286,
"startChar": 0,
"endLine": 286,
"endChar": 45
},
"revId": "ca24bc3e8d170ab9148a002dde8bdd1e3754249e",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "d11b576f_075b8ea1",
"filename": "octavia_tempest_plugin/tests/scenario/v2/test_backup_members.py",
"patchSetId": 11
},
"lineNbr": 317,
"author": {
"id": 28609
},
"writtenOn": "2023-10-25T16:16:08Z",
"side": 1,
"message": "Nit\nConsider adding \"PAUSED\" to constants.",
"range": {
"startLine": 317,
"startChar": 42,
"endLine": 317,
"endChar": 48
},
"revId": "ca24bc3e8d170ab9148a002dde8bdd1e3754249e",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "05edf8bb_98d165f8",
"filename": "octavia_tempest_plugin/tests/scenario/v2/test_backup_members.py",
"patchSetId": 11
},
"lineNbr": 359,
"author": {
"id": 28609
},
"writtenOn": "2023-10-25T16:16:08Z",
"side": 1,
"message": "You can use already existing \"ACTIVE\" from constants.",
"range": {
"startLine": 359,
"startChar": 42,
"endLine": 359,
"endChar": 48
},
"revId": "ca24bc3e8d170ab9148a002dde8bdd1e3754249e",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "26d18114_5a2e5471",
"filename": "octavia_tempest_plugin/tests/scenario/v2/test_backup_members.py",
"patchSetId": 11
},
"lineNbr": 360,
"author": {
"id": 28609
},
"writtenOn": "2023-10-25T16:16:08Z",
"side": 1,
"message": "The default timeout is set to 60, but I think that in terms of these kind of tests (backup should happen as fast as possible) we have to reduce it, maybe 10 sec?\nI\u0027d suggest asking Dev for proper timeout value to be used here.",
"range": {
"startLine": 360,
"startChar": 16,
"endLine": 360,
"endChar": 31
},
"revId": "ca24bc3e8d170ab9148a002dde8bdd1e3754249e",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}