x
/
kloudbuster
Code Issues Proposed changes

Update patch set 1 

Patch Set 1: Code-Review-1

(4 comments)

Please also fix the pep warnings.
Could you also check the case of timeout that the code behaves properly? For that you can put a big size and short timeout and verify that the run fails properly and all resources are cleaned.

Patch-set: 1
Label: Code-Review=-1
This commit is contained in:
Gerrit User 11744 2020-08-07 04:49:08 +00:00 committed by Gerrit Code Review
parent 4187134c5a
commit 1b7b2fa8cb
1 changed files with 69 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
a8eb6e962db4e941c78dd23c334b6dae814293b5
View File

@ -1,5 +1,22 @@
{
"comments": [
{
"key": {
"uuid": "9f560f44_5bc38157",
"filename": "kloudbuster/base_network.py",
"patchSetId": 1
},
"lineNbr": 176,
"author": {
"id": 11744
},
"writtenOn": "2020-08-07T04:49:08Z",
"side": 1,
"message": "the default value should be 150,\nalso it might be better to check that it is an integer here and if not integer fallback to the default value with a warning.\n\nDEFAULT_VOL_CREATE_TIMEOUT_SEC \u003d 150\nvol_timeout \u003d config_scale[\u0027storage_stage_configs\u0027].get(\u0027vol_create_timeout_sec\u0027, DEFAULT_VOL_CREATE_TIMEOUT_SEC)\ntry:\n vol_timeout \u003d int(config_scale[\u0027storage_stage_configs\u0027].get(\u0027vol_create_timeout_sec\u0027, DEFAULT_VOL_CREATE_TIMEOUT_SEC))\nexcept ValueError:\n vol_timeout \u003d DEFAULT_VOL_CREATE_TIMEOUT_SEC",
"revId": "a8eb6e962db4e941c78dd23c334b6dae814293b5",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_502fe086",
@ -18,6 +35,23 @@
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_3b97ad4a",
"filename": "kloudbuster/base_network.py",
"patchSetId": 1
},
"lineNbr": 196,
"author": {
"id": 11744
},
"writtenOn": "2020-08-07T04:49:08Z",
"side": 1,
"message": "I think you can collapse the 2 cases, get rid of if/else and just keep L193\nsince not passing type (L196) is same as passing type\u003dNone",
"revId": "a8eb6e962db4e941c78dd23c334b6dae814293b5",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_b04f5c69",
@ -36,6 +70,23 @@
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_dbacb1a0",
"filename": "kloudbuster/base_storage.py",
"patchSetId": 1
},
"lineNbr": 42,
"author": {
"id": 11744
},
"writtenOn": "2020-08-07T04:49:08Z",
"side": 1,
"message": "I think we can check more frequently like every 5s",
"revId": "a8eb6e962db4e941c78dd23c334b6dae814293b5",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_90521812",
@ -89,6 +140,24 @@
"revId": "a8eb6e962db4e941c78dd23c334b6dae814293b5",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "9f560f44_1b92a957",
"filename": "kloudbuster/base_storage.py",
"patchSetId": 1
},
"lineNbr": 49,
"author": {
"id": 11744
},
"writtenOn": "2020-08-07T04:49:08Z",
"side": 1,
"message": "in case of timeout we need to raise an exception with proper error message - otherwise the caller will continue as if no error happened.",
"parentUuid": "9f560f44_f05554fb",
"revId": "a8eb6e962db4e941c78dd23c334b6dae814293b5",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
}
]
}