Update patch set 2

Patch Set 2:

(4 comments)

Patch-set: 2
Attention: {"person_ident":"Gerrit User 22873 \u003c22873@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_30674\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 30674 \u003c30674@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_30674\u003e replied on the change"}

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"
     }
   ]
 }