charm-ceph-osd/23db141218d8a3cd0ff506f2a05c60db484034aa

{
  "comments": [
    {
      "unresolved": false,
      "key": {
        "uuid": "9c8a8331_5239ee23",
        "filename": "/PATCHSET_LEVEL",
        "patchSetId": 1
      },
      "lineNbr": 0,
      "author": {
        "id": 34352
      },
      "writtenOn": "2023-09-14T02:21:43Z",
      "side": 1,
      "message": "This is a follow-up to https://review.opendev.org/c/openstack/charm-ceph-osd/+/869619/",
      "revId": "23db141218d8a3cd0ff506f2a05c60db484034aa",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "c3ea78ad_76bf264c",
        "filename": "hooks/ceph_hooks.py",
        "patchSetId": 1
      },
      "lineNbr": 398,
      "author": {
        "id": 33717
      },
      "writtenOn": "2023-09-21T20:44:26Z",
      "side": 1,
      "message": "I think something like a static method for an ergonomic constructor could make things clearer. Something like:\n\n```\n@staticmethod\ndef make(value\u003d\"\", parsed_type\u003dOSDMemoryTargetType.EMPTY):\n  return OSDMemoryTarget(value\u003dvalue, parsed_type\u003dparsed_type)\n```\n\nAnd then use it below.",
      "revId": "23db141218d8a3cd0ff506f2a05c60db484034aa",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": false,
      "key": {
        "uuid": "01e89a7d_441603df",
        "filename": "hooks/ceph_hooks.py",
        "patchSetId": 1
      },
      "lineNbr": 398,
      "author": {
        "id": 34352
      },
      "writtenOn": "2023-09-29T04:12:05Z",
      "side": 1,
      "message": "Ack",
      "parentUuid": "c3ea78ad_76bf264c",
      "revId": "23db141218d8a3cd0ff506f2a05c60db484034aa",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "ebb40201_1adf1b0c",
        "filename": "hooks/ceph_hooks.py",
        "patchSetId": 1
      },
      "lineNbr": 412,
      "author": {
        "id": 33717
      },
      "writtenOn": "2023-09-21T20:44:26Z",
      "side": 1,
      "message": "Return type needs updating.",
      "revId": "23db141218d8a3cd0ff506f2a05c60db484034aa",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": false,
      "key": {
        "uuid": "22977c99_1f85e641",
        "filename": "hooks/ceph_hooks.py",
        "patchSetId": 1
      },
      "lineNbr": 412,
      "author": {
        "id": 34352
      },
      "writtenOn": "2023-09-29T04:12:05Z",
      "side": 1,
      "message": "Done",
      "parentUuid": "ebb40201_1adf1b0c",
      "revId": "23db141218d8a3cd0ff506f2a05c60db484034aa",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "c4c8203c_33272995",
        "filename": "hooks/ceph_hooks.py",
        "patchSetId": 1
      },
      "lineNbr": 441,
      "author": {
        "id": 34352
      },
      "writtenOn": "2023-09-15T03:00:40Z",
      "side": 1,
      "message": "Not sure how pythonic all this is, but it was fun playing with enums.  The alternative I guess would be to add a `is_valid_tune_osd_memory_target() -\u003e bool` function, but feels like repeating logic.",
      "revId": "23db141218d8a3cd0ff506f2a05c60db484034aa",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "0256237c_83a1a6e5",
        "filename": "hooks/ceph_hooks.py",
        "patchSetId": 1
      },
      "lineNbr": 441,
      "author": {
        "id": 15382
      },
      "writtenOn": "2023-09-18T11:41:40Z",
      "side": 1,
      "message": "\u003e Not sure how pythonic all this is, but it was fun playing with enums.  The alternative I guess would be to add a `is_valid_tune_osd_memory_target() -\u003e bool` function, but feels like repeating logic.\n\nI believe the enum type works fine here. \n\nPossibly could consider using a frozen dataclass instead of namedtuple here, and could move the parsing logic into a __post_init__() method, for some added safety and flexibility, but I wouldn\u0027t block over it.",
      "parentUuid": "c4c8203c_33272995",
      "revId": "23db141218d8a3cd0ff506f2a05c60db484034aa",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    },
    {
      "unresolved": false,
      "key": {
        "uuid": "1c36132e_f44df22b",
        "filename": "hooks/ceph_hooks.py",
        "patchSetId": 1
      },
      "lineNbr": 441,
      "author": {
        "id": 34352
      },
      "writtenOn": "2023-09-29T04:12:05Z",
      "side": 1,
      "message": "Ok, I dropped the playing with enums and namedtuples because it was getting too complicated.  Please see the latest update which adds a simple function to check validity in assess status.",
      "parentUuid": "0256237c_83a1a6e5",
      "revId": "23db141218d8a3cd0ff506f2a05c60db484034aa",
      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
    }
  ]
}