openstack
/
kuryr-kubernetes
Code Issues Proposed changes
kuryr-kubernetes/7ecb3e2ba2eabd65bcd564d5a43...

278 lines
9.0 KiB
Plaintext
Raw Blame History

{
"comments": [
{
"key": {
"uuid": "a4500fd5_f3bc5478",
"filename": "kuryr_kubernetes/controller/drivers/utils.py",
"patchSetId": 12
},
"lineNbr": 345,
"author": {
"id": 33240
},
"writtenOn": "2021-07-12T05:03:17Z",
"side": 1,
"message": "I have created this utility function to also replace the one defined at line 325. The name and the resource needs to be passed from the calling function.",
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "98592f1d_117a9d47",
"filename": "kuryr_kubernetes/controller/drivers/utils.py",
"patchSetId": 12
},
"lineNbr": 345,
"author": {
"id": 23567
},
"writtenOn": "2021-07-12T07:32:35Z",
"side": 1,
"message": "perhaps I did not explain myself properly here. My intention was to have the 2 functions you had before (get_kuryrnetworkpolicy_crds, get_kuryrloadbalancer_crds), but internally they will call something similar as what you have here (e.g., _get_crd_resource).",
"parentUuid": "a4500fd5_f3bc5478",
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "32f06e0b_80c67202",
"filename": "kuryr_kubernetes/controller/drivers/utils.py",
"patchSetId": 12
},
"lineNbr": 345,
"author": {
"id": 33240
},
"writtenOn": "2021-07-12T08:31:03Z",
"side": 1,
"message": "\u003e perhaps I did not explain myself properly here. My intention was to have the 2 functions you had before (get_kuryrnetworkpolicy_crds, get_kuryrloadbalancer_crds), but internally they will call something similar as what you have here (e.g., _get_crd_resource).\n\nOkay, If I read you correctly, I would need to have both functions previously and still have them call this _get_crd_resource function individually? I think that is correct?",
"parentUuid": "98592f1d_117a9d47",
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "9c83ac6f_093c74e5",
"filename": "kuryr_kubernetes/controller/drivers/utils.py",
"patchSetId": 12
},
"lineNbr": 349,
"author": {
"id": 27032
},
"writtenOn": "2021-07-12T08:59:04Z",
"side": 1,
"message": "Why does the resource need to be reassigned?\nBetter to directly change the name of resource parameter to resource_path instead.",
"range": {
"startLine": 349,
"startChar": 8,
"endLine": 349,
"endChar": 21
},
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "c0147a1a_b2925052",
"filename": "kuryr_kubernetes/controller/drivers/utils.py",
"patchSetId": 12
},
"lineNbr": 350,
"author": {
"id": 23567
},
"writtenOn": "2021-07-12T07:32:35Z",
"side": 1,
"message": "klbs is not the right name for the var name if this is for different type of resources. Also missing the namespace piece",
"range": {
"startLine": 350,
"startChar": 8,
"endLine": 350,
"endChar": 13
},
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "b4a5687a_66c95b05",
"filename": "kuryr_kubernetes/controller/drivers/utils.py",
"patchSetId": 12
},
"lineNbr": 358,
"author": {
"id": 27032
},
"writtenOn": "2021-07-12T08:59:04Z",
"side": 1,
"message": "might be safer to define klbs \u003d {} before the try/except block or move this return to line 351",
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "4aec6b0d_768698b2",
"filename": "kuryr_kubernetes/controller/handlers/loadbalancer.py",
"patchSetId": 12
},
"lineNbr": 137,
"author": {
"id": 27032
},
"writtenOn": "2021-07-12T08:59:04Z",
"side": 1,
"message": "This same function is called at line 172 also for KuryrLoadBalancers. They can be moved to outside (line 122) and pass the return as parameter of the trigger_* functions, avoiding to call k8s API twice.",
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "69298ff3_c844a1d9",
"filename": "kuryr_kubernetes/controller/handlers/loadbalancer.py",
"patchSetId": 12
},
"lineNbr": 178,
"author": {
"id": 23567
},
"writtenOn": "2021-07-12T07:32:35Z",
"side": 1,
"message": "perhaps this could be a dict instead, same for the loadbalancer reconciliation function",
"range": {
"startLine": 178,
"startChar": 24,
"endLine": 178,
"endChar": 76
},
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "8b881d9d_a4667bfc",
"filename": "kuryr_kubernetes/controller/handlers/loadbalancer.py",
"patchSetId": 12
},
"lineNbr": 178,
"author": {
"id": 27032
},
"writtenOn": "2021-07-12T08:59:04Z",
"side": 1,
"message": "+1",
"parentUuid": "69298ff3_c844a1d9",
"range": {
"startLine": 178,
"startChar": 24,
"endLine": 178,
"endChar": 76
},
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "a91889b5_2eb07743",
"filename": "kuryr_kubernetes/controller/handlers/loadbalancer.py",
"patchSetId": 12
},
"lineNbr": 188,
"author": {
"id": 23567
},
"writtenOn": "2021-07-12T07:32:35Z",
"side": 1,
"message": "perhaps this needs a bit extra thinking/checking. It is not enough the listener exists... for example the listeners must be associated to the loadbalancer we expect.",
"range": {
"startLine": 186,
"startChar": 1,
"endLine": 188,
"endChar": 69
},
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "b75850b8_6fe24b6c",
"filename": "kuryr_kubernetes/controller/handlers/loadbalancer.py",
"patchSetId": 12
},
"lineNbr": 188,
"author": {
"id": 33240
},
"writtenOn": "2021-07-12T08:31:03Z",
"side": 1,
"message": "\u003e perhaps this needs a bit extra thinking/checking. It is not enough the listener exists... for example the listeners must be associated to the loadbalancer we expect.\n\nI need to implement a check to confirm if the listener is associated with the right loadbalancer?",
"parentUuid": "a91889b5_2eb07743",
"range": {
"startLine": 186,
"startChar": 1,
"endLine": 188,
"endChar": 69
},
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "ccdddc5b_d94ee82e",
"filename": "kuryr_kubernetes/controller/handlers/loadbalancer.py",
"patchSetId": 12
},
"lineNbr": 222,
"author": {
"id": 23567
},
"writtenOn": "2021-07-12T07:32:35Z",
"side": 1,
"message": "this is exactly the same as the previous function",
"range": {
"startLine": 210,
"startChar": 1,
"endLine": 222,
"endChar": 22
},
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "70c725a2_e77ac18d",
"filename": "kuryr_kubernetes/controller/handlers/loadbalancer.py",
"patchSetId": 12
},
"lineNbr": 222,
"author": {
"id": 27032
},
"writtenOn": "2021-07-12T08:59:04Z",
"side": 1,
"message": "+1 no need to have this extra function",
"parentUuid": "ccdddc5b_d94ee82e",
"range": {
"startLine": 210,
"startChar": 1,
"endLine": 222,
"endChar": 22
},
"revId": "7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
}
]
}
View Git Blame Copy Permalink