python-tempestconf/aa037e9ce093e87a7b22000c401...

160 lines
5.7 KiB
Plaintext

{
"comments": [
{
"unresolved": false,
"key": {
"uuid": "4a531354_ea420ed5",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 2
},
"lineNbr": 0,
"author": {
"id": 35007
},
"writtenOn": "2023-11-29T03:38:35Z",
"side": 1,
"message": "Hi Martin,\n\nIn our cloud, we\u0027ve configured our endpoints to have path based suffixes (e.g. `http://IP:PORT/\u003csome_path\u003e/v2`), and the discovery tool isn\u0027t working properly with this kind of endpoints because it cuts too much (`\u003csome_path\u003e/v2` this is example) when the port is **not** None. I tried to work around this by using `utils.get_base_url` instead of `no_port_cut_url` to properly remove the version in the service URL, this should also be backward-compatible. \n\nCan I ask you opinion regarding this approach / issue?\n\nThank you.",
"revId": "aa037e9ce093e87a7b22000c4013be247c645b71",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": false,
"key": {
"uuid": "4ef014d5_9412aa54",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 2
},
"lineNbr": 0,
"author": {
"id": 32363
},
"writtenOn": "2023-11-29T21:43:48Z",
"side": 1,
"message": "These changes seem reasonable to me.",
"revId": "aa037e9ce093e87a7b22000c4013be247c645b71",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": false,
"key": {
"uuid": "5792922a_05c60e16",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 2
},
"lineNbr": 0,
"author": {
"id": 22873
},
"writtenOn": "2023-11-30T13:02:00Z",
"side": 1,
"message": "looks reasonable and the gates seem to agree",
"revId": "aa037e9ce093e87a7b22000c4013be247c645b71",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": false,
"key": {
"uuid": "cbae9b89_70d0c807",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 2
},
"lineNbr": 0,
"author": {
"id": 22873
},
"writtenOn": "2023-11-30T13:02:07Z",
"side": 1,
"message": "check experimental",
"revId": "aa037e9ce093e87a7b22000c4013be247c645b71",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": false,
"key": {
"uuid": "6a265ffb_e6c1d457",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 2
},
"lineNbr": 0,
"author": {
"id": 10459
},
"writtenOn": "2023-11-30T13:28:28Z",
"side": 1,
"message": "Does this affect other services too? I see share.py which follows the same pattern, but I haven\u0027t checked.\n\nWould it make sense to add unit tests?\nAnd a release note?",
"revId": "aa037e9ce093e87a7b22000c4013be247c645b71",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": false,
"key": {
"uuid": "54f42403_2af2e22a",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 2
},
"lineNbr": 0,
"author": {
"id": 35007
},
"writtenOn": "2023-12-01T03:31:37Z",
"side": 1,
"message": "\u003e Does this affect other services too?\n\nPossibly yes, but in our cloud we don\u0027t have `ShareService` so I did not see it as part of the story.\n\n\u003e Would it make sense to add unit tests?\n\nYes, I initially thought of adding one, but not exactly sure how can I do it easily without copy-and-pasting the test because the test is intended to test *one* possible pattern of [FAKE_URL](https://opendev.org/openinfra/python-tempestconf/src/branch/master/config_tempest/tests/services/test_compute.py#L28), do you happen to have some suggestions?\n\n\u003e And a release note?\n\nI am assuming it\u0027s for the maintainer?",
"parentUuid": "6a265ffb_e6c1d457",
"revId": "aa037e9ce093e87a7b22000c4013be247c645b71",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": false,
"key": {
"uuid": "65c1ea69_a5626f67",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 2
},
"lineNbr": 0,
"author": {
"id": 35007
},
"writtenOn": "2023-12-07T06:05:45Z",
"side": 1,
"message": "would appreciate any follow up on this, thanks!",
"revId": "aa037e9ce093e87a7b22000c4013be247c645b71",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "4cad41a6_bed2ff87",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 2
},
"lineNbr": 0,
"author": {
"id": 30674
},
"writtenOn": "2023-12-11T08:47:48Z",
"side": 1,
"message": "I think that if this issue influences other services (ShareService) we should fix it for this service as well (if it is an easy fix, which it seems to be in this case).",
"parentUuid": "54f42403_2af2e22a",
"revId": "aa037e9ce093e87a7b22000c4013be247c645b71",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
},
{
"unresolved": true,
"key": {
"uuid": "25c8b899_cb9349f8",
"filename": "/PATCHSET_LEVEL",
"patchSetId": 2
},
"lineNbr": 0,
"author": {
"id": 35007
},
"writtenOn": "2023-12-12T07:28:08Z",
"side": 1,
"message": "I\u0027ve included the fix for share service, as well as some unit test for `get_base_url` since it\u0027s the primary function used the `set_versions` method",
"parentUuid": "4cad41a6_bed2ff87",
"revId": "aa037e9ce093e87a7b22000c4013be247c645b71",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
}
]
}