diff --git a/834afdd6b1440d3696dcc27c9c3de04a1620ea01 b/834afdd6b1440d3696dcc27c9c3de04a1620ea01 index f9744476..1878c1d5 100644 --- a/834afdd6b1440d3696dcc27c9c3de04a1620ea01 +++ b/834afdd6b1440d3696dcc27c9c3de04a1620ea01 @@ -16,6 +16,92 @@ "message": "recheck quite strange error, wondering what has changed and where", "revId": "834afdd6b1440d3696dcc27c9c3de04a1620ea01", "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" + }, + { + "unresolved": false, + "key": { + "uuid": "fb9c8183_6c2ffcd1", + "filename": "/PATCHSET_LEVEL", + "patchSetId": 2 + }, + "lineNbr": 0, + "author": { + "id": 30674 + }, + "writtenOn": "2023-09-14T11:20:58Z", + "side": 1, + "message": "I added a few comments. The function is used only in two places in python_tempestconf but I think that the function should work well on its own if we ever want to use it somewhere else. I would at least update the docstring or add the logic before line 49.", + "revId": "834afdd6b1440d3696dcc27c9c3de04a1620ea01", + "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" + }, + { + "unresolved": true, + "key": { + "uuid": "137bd201_b09b937f", + "filename": "config_tempest/users.py", + "patchSetId": 2 + }, + "lineNbr": 51, + "author": { + "id": 30674 + }, + "writtenOn": "2023-09-14T11:20:58Z", + "side": 1, + "message": "nit: list or roles_list?", + "range": { + "startLine": 51, + "startChar": 46, + "endLine": 51, + "endChar": 49 + }, + "revId": "834afdd6b1440d3696dcc27c9c3de04a1620ea01", + "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" + }, + { + "unresolved": true, + "key": { + "uuid": "24c18990_07188b9a", + "filename": "config_tempest/users.py", + "patchSetId": 2 + }, + "lineNbr": 74, + "author": { + "id": 30674 + }, + "writtenOn": "2023-09-14T11:20:58Z", + "side": 1, + "message": "Is it OK?\n```\n# This will lead to the function assigning user \"Admin\" role \"admin\" if \n# \"tortuga\" user does not exist on the system. It might be unwanted side effect.\ngive_role_to_user(username\u003d\"tortuga\", role\u003d\"admin\")\n```", + "range": { + "startLine": 74, + "startChar": 0, + "endLine": 74, + "endChar": 1 + }, + "revId": "834afdd6b1440d3696dcc27c9c3de04a1620ea01", + "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" + }, + { + "unresolved": true, + "key": { + "uuid": "6269652e_8ce96e93", + "filename": "config_tempest/users.py", + "patchSetId": 2 + }, + "lineNbr": 86, + "author": { + "id": 30674 + }, + "writtenOn": "2023-09-14T11:20:58Z", + "side": 1, + "message": "I\u0027m not sure but am I reading this correctly that we might assign a role \"Admin\" to a user if a given role is not found on the system. For example:\n\n```\n# This will lead to \"user1\" being assigned role \"Admin\" if it is present on the \n# system despite the user of the function wanted role \"tortuga\" \ngive_role_to_user(username\u003d\"user1\", role_name\u003d\"tortuga\")\n```\n\nI do not know how likely this scenario is. But I would at least update the docstring for the function.", + "range": { + "startLine": 86, + "startChar": 8, + "endLine": 86, + "endChar": 24 + }, + "revId": "834afdd6b1440d3696dcc27c9c3de04a1620ea01", + "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" } ] } \ No newline at end of file