Update patch set 22

Patch Set 22: Code-Review-1

(4 comments)

Most generally, I think you are adding too much complexity for this.
If we want to get rid of access rights and directory creation in /var/log for the VF side and want to start to write log in a dedicated directory (/home/$user/validations/), this review should only adds the bits for managing this directory:
1/ create if not exist
2/ check write access
3/ raise if unable to read/write and stop.

* The constants.py should be the default.
* Just one function in utils.py that does the creation, check rights and raise.
* The log path option exposed in the CLI with a default value to the constants.py for the run command (history already get an option for that).

The user can also have to possibility to provide his own path, so then tripleo can override with the classic /var/log/validation (or not).

Regarding the callback, since during the execution the callback can not have access to the current location in used, I think the run code should export an environment variable with the current path in used... or we can look to the ansible side if there is a variable exported with this information.

Patch-set: 22
Reviewer: Gerrit User 16515 <16515@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1
This commit is contained in:
Gerrit User 16515 2021-05-05 06:27:15 +00:00 committed by Gerrit Code Review
parent 074d25616e
commit 44debfed8f
1 changed files with 92 additions and 0 deletions

View File

@ -51,6 +51,29 @@
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "0574cacb_dd140e9e",
"filename": "validations_libs/utils.py",
"patchSetId": 21
},
"lineNbr": 35,
"author": {
"id": 16515
},
"writtenOn": "2021-05-05T06:27:15Z",
"side": 1,
"message": "You should drop this function, which doesn\u0027t really make sense.\nIf we want to check the presence of /tmp in the path, we dont need a specific function for it",
"range": {
"startLine": 35,
"startChar": 0,
"endLine": 35,
"endChar": 29
},
"revId": "2b50ae5bab3fbe573afef6ba9323d235fb76f5b3",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "1d7a5be8_4c3b44be",
@ -119,6 +142,75 @@
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "3b0fab81_d28d1c75",
"filename": "validations_libs/utils.py",
"patchSetId": 21
},
"lineNbr": 50,
"author": {
"id": 16515
},
"writtenOn": "2021-05-05T06:27:15Z",
"side": 1,
"message": "If we assume that now the default directory for the log is:\n$HOME/validations\n\nthen this function should just:\n1/ check if this path is present\n2/ if it\u0027s writable\n3/ if not present try to create it.",
"range": {
"startLine": 50,
"startChar": 0,
"endLine": 50,
"endChar": 29
},
"revId": "2b50ae5bab3fbe573afef6ba9323d235fb76f5b3",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "dd18c49c_f8a7bc5f",
"filename": "validations_libs/utils.py",
"patchSetId": 21
},
"lineNbr": 89,
"author": {
"id": 16515
},
"writtenOn": "2021-05-05T06:27:15Z",
"side": 1,
"message": "You should revert to the master code here.\nYou are doing almost the same behavior.",
"range": {
"startLine": 89,
"startChar": 4,
"endLine": 89,
"endChar": 43
},
"revId": "2b50ae5bab3fbe573afef6ba9323d235fb76f5b3",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "d2c95393_485f3b57",
"filename": "validations_libs/utils.py",
"patchSetId": 21
},
"lineNbr": 145,
"author": {
"id": 16515
},
"writtenOn": "2021-05-05T06:27:15Z",
"side": 1,
"message": "this should be done in one place, only one time, otherwise you will get race where in some case it will fallback to tmp and some other not.",
"range": {
"startLine": 145,
"startChar": 18,
"endLine": 145,
"endChar": 77
},
"revId": "2b50ae5bab3fbe573afef6ba9323d235fb76f5b3",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": true
},
{
"key": {
"uuid": "68e931c0_d3968e97",