openstack
/
kuryr-kubernetes
Code Issues Proposed changes
kuryr-kubernetes/6fe7ddbd5e030589eb14f5d3a7c...

264 lines
8.3 KiB
Plaintext
Raw Blame History

{
"comments": [
{
"key": {
"uuid": "1f1a1f67_acce72b1",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 49,
"author": {
"id": 6598
},
"writtenOn": "2017-07-17T06:28:02Z",
"side": 1,
"message": "maybe rename to complete_processing?",
"range": {
"startLine": 49,
"startChar": 8,
"endLine": 49,
"endChar": 22
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "1f1a1f67_0fdf980d",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 49,
"author": {
"id": 15168
},
"writtenOn": "2017-07-17T07:13:29Z",
"side": 1,
"message": "I can change that, but it would not be accurate.\n\nin a sense calling the callback (which is always _done for CNI) completes processing. So I factored this function out to determine if we need to call the callback or not. \n\nActually we can change it a bit more, so that on_present calls the callback. Then the function would have to return True/False and might be more obvious for the reader",
"parentUuid": "1f1a1f67_acce72b1",
"range": {
"startLine": 49,
"startChar": 8,
"endLine": 49,
"endChar": 22
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "1f1a1f67_e23d938f",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 49,
"author": {
"id": 6598
},
"writtenOn": "2017-07-17T07:42:58Z",
"side": 1,
"message": "+1 for changing the code to call callback by on_present",
"parentUuid": "1f1a1f67_0fdf980d",
"range": {
"startLine": 49,
"startChar": 8,
"endLine": 49,
"endChar": 22
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "1f1a1f67_914067a7",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 49,
"author": {
"id": 15168
},
"writtenOn": "2017-07-17T10:50:51Z",
"side": 1,
"message": "ah, the problem is that we\u0027ll be leaking AddHandler-specific code into BaseHandler. That was the reason I split this function in the first place.\nAddHandler needs to call callback with vif as parameter, DelHandler doesn\u0027t \u003d/\nI can factor that out too, but that feels like over-engineering. let\u0027s see how that works out though.",
"parentUuid": "1f1a1f67_e23d938f",
"range": {
"startLine": 49,
"startChar": 8,
"endLine": 49,
"endChar": 22
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "1f1a1f67_4ce47e3c",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 56,
"author": {
"id": 6598
},
"writtenOn": "2017-07-17T06:28:02Z",
"side": 1,
"message": "returns?",
"range": {
"startLine": 56,
"startChar": 6,
"endLine": 56,
"endChar": 11
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "1f1a1f67_cfe400e3",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 56,
"author": {
"id": 15168
},
"writtenOn": "2017-07-17T07:13:29Z",
"side": 1,
"message": "will change/add",
"parentUuid": "1f1a1f67_4ce47e3c",
"range": {
"startLine": 56,
"startChar": 6,
"endLine": 56,
"endChar": 11
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "1f1a1f67_ac431213",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 114,
"author": {
"id": 6598
},
"writtenOn": "2017-07-17T06:28:02Z",
"side": 1,
"message": "I am not sure you can control when this will be called. Please put in docstring the details of the method",
"range": {
"startLine": 114,
"startChar": 7,
"endLine": 114,
"endChar": 52
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "1f1a1f67_6f3dec29",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 114,
"author": {
"id": 15168
},
"writtenOn": "2017-07-17T07:13:29Z",
"side": 1,
"message": "I don\u0027t really understand what you mean. the function is always called on L46. after all vifs have been processed. However if we change the semantics as I mentioned above the docstring would change also.",
"parentUuid": "1f1a1f67_ac431213",
"range": {
"startLine": 114,
"startChar": 7,
"endLine": 114,
"endChar": 52
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "1f1a1f67_a2df9b38",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 114,
"author": {
"id": 6598
},
"writtenOn": "2017-07-17T07:42:58Z",
"side": 1,
"message": "My comment is mostly about the content of the docstring.\nI mean that while you can assume that the method will be called at certain point, you cannot guarantee this. I think docstring of the method is not the place to reflect the assumption on when the method will be called. If your precondition is that all vif are processed, this should be verified.",
"parentUuid": "1f1a1f67_6f3dec29",
"range": {
"startLine": 114,
"startChar": 7,
"endLine": 114,
"endChar": 52
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "1f1a1f67_d1801fc2",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 114,
"author": {
"id": 15168
},
"writtenOn": "2017-07-17T10:50:51Z",
"side": 1,
"message": "I still don\u0027t get you. in the current form the method is always called *unconditionally* on L46. this method then determines if all vifs are active and calls (or doesn\u0027t) the callback.\nAnyway I\u0027ll factor out the logic into smaller chunks so this will change a bit.",
"parentUuid": "1f1a1f67_a2df9b38",
"range": {
"startLine": 114,
"startChar": 7,
"endLine": 114,
"endChar": 52
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "1f1a1f67_cc3e4e8a",
"filename": "kuryr_kubernetes/cni/handlers.py",
"patchSetId": 12
},
"lineNbr": 145,
"author": {
"id": 6598
},
"writtenOn": "2017-07-17T06:28:02Z",
"side": 1,
"message": "I am not sure you can control when this will be called. Please put in docstring the details of the method",
"range": {
"startLine": 145,
"startChar": 7,
"endLine": 145,
"endChar": 52
},
"revId": "6fe7ddbd5e030589eb14f5d3a7cc6a9d3cc21291",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
}
]
}
View Git Blame Copy Permalink