python-tempestconf/5ffde61b69cf79547c9cd1da731b62d99da0c762

{
  "comments": [
    {
      "unresolved": false,
      "key": {
        "uuid": "f3e6a99d_086c5077",
        "filename": "/PATCHSET_LEVEL",
        "patchSetId": 1
      },
      "lineNbr": 0,
      "author": {
        "id": 22873
      },
      "writtenOn": "2022-03-28T14:02:37Z",
      "side": 1,
      "message": "Thanks Tzach for finding the bug",
      "revId": "5ffde61b69cf79547c9cd1da731b62d99da0c762",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": false,
      "key": {
        "uuid": "7b48037c_c124269d",
        "filename": "/PATCHSET_LEVEL",
        "patchSetId": 1
      },
      "lineNbr": 0,
      "author": {
        "id": 21187
      },
      "writtenOn": "2022-03-29T06:54:27Z",
      "side": 1,
      "message": "makedirs showed up on the error that was a given, but how did you deduce from that it was a folder path issue? \n\nIs this related (or not) to the fact that I ran it on a BM with a lot of resources?",
      "parentUuid": "f3e6a99d_086c5077",
      "revId": "5ffde61b69cf79547c9cd1da731b62d99da0c762",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": false,
      "key": {
        "uuid": "0a0dd36a_1bf85400",
        "filename": "/PATCHSET_LEVEL",
        "patchSetId": 1
      },
      "lineNbr": 0,
      "author": {
        "id": 21187
      },
      "writtenOn": "2022-03-29T06:54:27Z",
      "side": 1,
      "message": "Thanks for looking into this issue and fixing it.  ",
      "revId": "5ffde61b69cf79547c9cd1da731b62d99da0c762",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": false,
      "key": {
        "uuid": "3736af5a_567a826e",
        "filename": "/PATCHSET_LEVEL",
        "patchSetId": 1
      },
      "lineNbr": 0,
      "author": {
        "id": 22873
      },
      "writtenOn": "2022-03-29T09:04:25Z",
      "side": 1,
      "message": "the issue wasn\u0027t related to the fact you have many resources.\n\nwell, the test was complaining that makedirs wasn\u0027t called as the test expected, so I went line by line in the test and in the tested method and found an if statement which called the makedirs if the provided path didn\u0027t exist - then i checked what path we use in the tests and found out that /img/ which is used by the tests actually exists on your environment, which lead to the fact that makedirs wasn\u0027t executed ...",
      "parentUuid": "7b48037c_c124269d",
      "revId": "5ffde61b69cf79547c9cd1da731b62d99da0c762",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": false,
      "key": {
        "uuid": "04a312f6_0a3095ef",
        "filename": "/PATCHSET_LEVEL",
        "patchSetId": 1
      },
      "lineNbr": 0,
      "author": {
        "id": 10459
      },
      "writtenOn": "2022-03-29T10:02:58Z",
      "side": 1,
      "message": "Can\u0027t it happen again if you have the directory with that name?\nWouldn\u0027t be it enough to pass exist_ok\u003dFalse to the makedirs call?",
      "revId": "5ffde61b69cf79547c9cd1da731b62d99da0c762",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": false,
      "key": {
        "uuid": "7fe80290_4e02cdf0",
        "filename": "/PATCHSET_LEVEL",
        "patchSetId": 1
      },
      "lineNbr": 0,
      "author": {
        "id": 22873
      },
      "writtenOn": "2022-03-29T12:03:48Z",
      "side": 1,
      "message": "it can, but what are the chances that dir like that will exist? This change affects only unit tests, the code is good and works as expected, the bug was only in the unit test.\n\nexist_ok\u003dFlase is the default:\nhttps://docs.python.org/3/library/os.html#os.makedirs\n\nI see 2 alternatives:\n1. using uuid module to generate a random string which can be used as a path, however, this requires a new import\n2. removing assert_called from the test - https://opendev.org/openinfra/python-tempestconf/src/commit/9586241409a61061d1a2a0bc1fc05192fdce8c13/config_tempest/tests/services/test_image.py#L61 which will remove unit test coverage for this part - https://opendev.org/openinfra/python-tempestconf/src/commit/9586241409a61061d1a2a0bc1fc05192fdce8c13/config_tempest/services/image.py#L133",
      "parentUuid": "04a312f6_0a3095ef",
      "revId": "5ffde61b69cf79547c9cd1da731b62d99da0c762",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": false,
      "key": {
        "uuid": "67511a47_358ed329",
        "filename": "/PATCHSET_LEVEL",
        "patchSetId": 1
      },
      "lineNbr": 0,
      "author": {
        "id": 10459
      },
      "writtenOn": "2022-03-29T12:28:05Z",
      "side": 1,
      "message": "Sorry, I meant exist_ok\u003dTrue\n\nThat way the test should work if the directory exists.",
      "parentUuid": "7fe80290_4e02cdf0",
      "revId": "5ffde61b69cf79547c9cd1da731b62d99da0c762",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    }
  ]
}