115 lines
3.9 KiB
Plaintext
115 lines
3.9 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "1f0912d1_1fa1ae7d",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 11
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 28522
|
|
},
|
|
"writtenOn": "2023-05-23T08:55:53Z",
|
|
"side": 1,
|
|
"message": "Hello,\n\nFirst thanks for this patch and for all your recent contributions.\n\nSee my inline suggestion.\nAlso I\u0027d suggest you to add some related docs with an example of usecase where user should think about enabling this feature.\n\nElse looks good to me.",
|
|
"revId": "196c9b53145e38a82ed2eead53b4d4aef3a8de66",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "fc20d807_25490dc6",
|
|
"filename": "osprofiler/drivers/otlp.py",
|
|
"patchSetId": 11
|
|
},
|
|
"lineNbr": 91,
|
|
"author": {
|
|
"id": 9816
|
|
},
|
|
"writtenOn": "2023-05-23T03:00:34Z",
|
|
"side": 1,
|
|
"message": "This is a breaking change which we should generally avoid.\n\nHowever otlp driver was added quite recently and we have never created a release since then. So updating this has no user impact.",
|
|
"range": {
|
|
"startLine": 91,
|
|
"startChar": 20,
|
|
"endLine": 91,
|
|
"endChar": 25
|
|
},
|
|
"revId": "196c9b53145e38a82ed2eead53b4d4aef3a8de66",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "a42f18cf_077dc303",
|
|
"filename": "osprofiler/drivers/otlp.py",
|
|
"patchSetId": 11
|
|
},
|
|
"lineNbr": 91,
|
|
"author": {
|
|
"id": 7730
|
|
},
|
|
"writtenOn": "2023-05-23T07:23:38Z",
|
|
"side": 1,
|
|
"message": "Good point.",
|
|
"parentUuid": "fc20d807_25490dc6",
|
|
"range": {
|
|
"startLine": 91,
|
|
"startChar": 20,
|
|
"endLine": 91,
|
|
"endChar": 25
|
|
},
|
|
"revId": "196c9b53145e38a82ed2eead53b4d4aef3a8de66",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "d500c2c4_524c120b",
|
|
"filename": "osprofiler/requests.py",
|
|
"patchSetId": 11
|
|
},
|
|
"lineNbr": 69,
|
|
"author": {
|
|
"id": 28522
|
|
},
|
|
"writtenOn": "2023-05-23T08:55:53Z",
|
|
"side": 1,
|
|
"message": "Just a `reader` comment.\n\nCan\u0027t we do that directly from line 63?\nThat\u0027s a bit redundant with what its did in the else section of the above `try/except`, and as `else` part is only executed if the `try` part is successful and that the import of `requests` is also successful, so I\u0027d suggest to init everything in the `else` part.\n\nThe `enable` function part would be only the confirmation to the user that http adapter is fully ready to be used by checking the state of `_FUNC` as you already did.",
|
|
"range": {
|
|
"startLine": 69,
|
|
"startChar": 8,
|
|
"endLine": 69,
|
|
"endChar": 31
|
|
},
|
|
"revId": "196c9b53145e38a82ed2eead53b4d4aef3a8de66",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "5a809511_140a124a",
|
|
"filename": "osprofiler/requests.py",
|
|
"patchSetId": 11
|
|
},
|
|
"lineNbr": 69,
|
|
"author": {
|
|
"id": 9816
|
|
},
|
|
"writtenOn": "2023-06-06T08:19:34Z",
|
|
"side": 1,
|
|
"message": "I think the point is that we need the warning message in case the user explicitly requests the profiling for requests. Because the whole enable method is called by initializer according to the config knob I think it makes sense to have this logic within enable method.",
|
|
"parentUuid": "d500c2c4_524c120b",
|
|
"range": {
|
|
"startLine": 69,
|
|
"startChar": 8,
|
|
"endLine": 69,
|
|
"endChar": 31
|
|
},
|
|
"revId": "196c9b53145e38a82ed2eead53b4d4aef3a8de66",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
}
|
|
]
|
|
} |