validations-libs/0dca0036744245e525ad1e1f0d1d55a8e69fbd64

{
  "comments": [
    {
      "key": {
        "uuid": "38f6fe1e_6127f682",
        "filename": "validations_libs/utils.py",
        "patchSetId": 32
      },
      "lineNbr": 207,
      "author": {
        "id": 16515
      },
      "writtenOn": "2021-05-10T09:04:33Z",
      "side": 1,
      "message": "I think all of this could be simplified with the original function.\nmakedir is recursive, so if the VF is creating:\nartifacrs/\u003cuuid\u003e\nit will create both at the same time.\nIf artifacts already exists it will create only \u003cuuid\u003e.\n\nThe original function return as well the newly created directory and uuid, so no need to create a getter function there, since it\u0027s called and needed only 1 time.\n\nSame for the marker, when we comes to the Ansible artifacts creation, we assume that the log directory is writable and accessible, so no need to ensure that at this stage.",
      "range": {
        "startLine": 83,
        "startChar": 0,
        "endLine": 207,
        "endChar": 41
      },
      "revId": "0dca0036744245e525ad1e1f0d1d55a8e69fbd64",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
      "unresolved": true
    },
    {
      "key": {
        "uuid": "8d8b5f1a_d0f9c64b",
        "filename": "validations_libs/utils.py",
        "patchSetId": 32
      },
      "lineNbr": 207,
      "author": {
        "id": 32926
      },
      "writtenOn": "2021-05-10T10:36:17Z",
      "side": 1,
      "message": "I\u0027m not an enemy of simplicity. And it is true, that amalgamation of these is possible.\n\nBut previous approach, albeit simple, also proved error prone and hard to debug.\n\nWe are not trying to accomplish a single task here, although it may seem to be the case.\nThe end result we seek, that is: writing validation ansible artifacts into a proper dir, along with logs one level above it, requires three steps to be taken.\n\n1. Ensure we have logs dir to write to.\n2. Ensure we have logs/artifacts dir to write to.\n3. Generating uuid and ensuring we have logs/artifacts/\u003cUUID\u003e_something dir to write to.\n\nAll three are an IO operations, during which we can encounter various issues and which are, by definition, comparably slow.\n\nThe separation into several functions minimizes chance for confusion, by providing each with its own logging and exception handling. This way we can immediately recognize which is causing issues and proceed accordingly. \n\nImplementing all as single function would entail creation of several conditional check, increasing the complexity to a substantial degree.\n\nFunctions creating the actual directories were factored out in order to keep the try/except blocks simple and to minimize code repetition. For example the `_setup_artifacts_dir` contains code that would otherwise have to be duplicated. And placed once in the try, and again in the except clause. \n\nThe get functions serve as, essentially, wrappers for the functions creating directories. Providing the exception handling on top.\n\nPreviously we were checking state of the logs/artifacts dir during each loop iteration, and in two different places, the utils.py and the constants.py. Now we only do it once, before entering the loop. And we do it only in utils.py, as the constants.py logic was removed. Making constants.py more like true constants.\n\nThe additional checks in all three of these steps, were implemented to precisely because the assumptions about filesystem don\u0027t have to hold. For example, the selected log dir may exist, but it may have wrong permissions. Only once we create the logs/artifacts dir can we be sure it\u0027s accessible to the current user.\n\nI can combine the steps 1 and 2, but most of the functions really ought to stay. Otherwise we will return to previous problems. Only magnified, by us newly letting the user pick his log folder in the command line.",
      "parentUuid": "38f6fe1e_6127f682",
      "range": {
        "startLine": 83,
        "startChar": 0,
        "endLine": 207,
        "endChar": 41
      },
      "revId": "0dca0036744245e525ad1e1f0d1d55a8e69fbd64",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
      "unresolved": true
    }
  ]
}