{ "comments": [ { "unresolved": false, "key": { "uuid": "03d011b3_7b7e9d44", "filename": "/PATCHSET_LEVEL", "patchSetId": 5 }, "lineNbr": 0, "author": { "id": 20870 }, "writtenOn": "2023-10-17T14:44:45Z", "side": 1, "message": "Thanks for the patch; I\u0027ve added some comments to the code, but the main thing that is needed is a zaza openstack test to verify that the certificate can be generated. Please could you look into adding one, and using `func-test-pr` in the commit message to cause zosci to pick up the test? Thanks.", "revId": "96f98b33188ed156f01e98b2560c5240e38bee4f", "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" }, { "unresolved": true, "key": { "uuid": "0a5f9264_e9d56881", "filename": "src/lib/charm/vault_pki.py", "patchSetId": 5 }, "lineNbr": 115, "author": { "id": 20870 }, "writtenOn": "2023-10-17T14:44:45Z", "side": 1, "message": "This `rtype` isn\u0027t valid? Doesn\u0027t the function return a Dict now?", "range": { "startLine": 115, "startChar": 5, "endLine": 115, "endChar": 17 }, "revId": "96f98b33188ed156f01e98b2560c5240e38bee4f", "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" }, { "unresolved": false, "key": { "uuid": "6feefa9d_43d503e8", "filename": "src/lib/charm/vault_pki.py", "patchSetId": 5 }, "lineNbr": 115, "author": { "id": 31503 }, "writtenOn": "2023-10-17T18:42:51Z", "side": 1, "message": "Done", "parentUuid": "0a5f9264_e9d56881", "range": { "startLine": 115, "startChar": 5, "endLine": 115, "endChar": 17 }, "revId": "96f98b33188ed156f01e98b2560c5240e38bee4f", "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" }, { "unresolved": true, "key": { "uuid": "aff128d5_f8d0c53c", "filename": "src/lib/charm/vault_pki.py", "patchSetId": 5 }, "lineNbr": 126, "author": { "id": 20870 }, "writtenOn": "2023-10-17T14:44:45Z", "side": 1, "message": "What will the cert_type be here if it\u0027s not \u0027server\u0027 or \u0027client\u0027 with respect to intermediate certificates? It would be good to check that and then continue to raise the unsupported cert_type. e.g. I suspect \"intermediate\" as in below?\n\nI know there is the check lower down, but that splits the \u0027validate the cert_type\u0027 into two places in the code which makes it harder for maintenance later.", "range": { "startLine": 126, "startChar": 0, "endLine": 126, "endChar": 0 }, "revId": "96f98b33188ed156f01e98b2560c5240e38bee4f", "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" }, { "unresolved": false, "key": { "uuid": "5c8412c2_7f93270c", "filename": "src/lib/charm/vault_pki.py", "patchSetId": 5 }, "lineNbr": 126, "author": { "id": 31503 }, "writtenOn": "2023-10-17T18:42:51Z", "side": 1, "message": "Done", "parentUuid": "aff128d5_f8d0c53c", "range": { "startLine": 126, "startChar": 0, "endLine": 126, "endChar": 0 }, "revId": "96f98b33188ed156f01e98b2560c5240e38bee4f", "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" }, { "unresolved": true, "key": { "uuid": "818b69b6_981fe612", "filename": "src/lib/charm/vault_pki.py", "patchSetId": 5 }, "lineNbr": 145, "author": { "id": 20870 }, "writtenOn": "2023-10-17T14:44:45Z", "side": 1, "message": "Why not combine this into the \u0027cert_type\u0027 checking above, and move the config setup to before that section. e.g.\n\n config \u003d ...\n if cert_type in (\u0027server\u0027, \u0027client\u0027):\n response \u003d ....(\n role\u003dCHARM_PKI_ROLE if cert_type \u003d\u003d \u0027server\u0027 else CHARM_PKI_MP,\n ...\n )\n ...\n \nNow you don\u0027t have to split the decision on the \u0027role\u0027 around the config setup.", "range": { "startLine": 135, "startChar": 8, "endLine": 145, "endChar": 45 }, "revId": "96f98b33188ed156f01e98b2560c5240e38bee4f", "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" }, { "unresolved": false, "key": { "uuid": "5e10e53c_44da5c6d", "filename": "src/lib/charm/vault_pki.py", "patchSetId": 5 }, "lineNbr": 145, "author": { "id": 31503 }, "writtenOn": "2023-10-17T18:42:51Z", "side": 1, "message": "Done", "parentUuid": "818b69b6_981fe612", "range": { "startLine": 135, "startChar": 8, "endLine": 145, "endChar": 45 }, "revId": "96f98b33188ed156f01e98b2560c5240e38bee4f", "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" }, { "unresolved": true, "key": { "uuid": "fc528f9b_d936a84f", "filename": "src/lib/charm/vault_pki.py", "patchSetId": 5 }, "lineNbr": 146, "author": { "id": 20870 }, "writtenOn": "2023-10-17T14:44:45Z", "side": 1, "message": "I\u0027d do \"return response[\u0027data\u0027]\" here to signal that this is the end of processing \u0027server\u0027 and \u0027client\u0027 cert_types.", "range": { "startLine": 146, "startChar": 1, "endLine": 146, "endChar": 2 }, "revId": "96f98b33188ed156f01e98b2560c5240e38bee4f", "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" }, { "unresolved": false, "key": { "uuid": "56516b43_6a3d6f08", "filename": "src/lib/charm/vault_pki.py", "patchSetId": 5 }, "lineNbr": 146, "author": { "id": 31503 }, "writtenOn": "2023-10-17T18:42:51Z", "side": 1, "message": "Done", "parentUuid": "fc528f9b_d936a84f", "range": { "startLine": 146, "startChar": 1, "endLine": 146, "endChar": 2 }, "revId": "96f98b33188ed156f01e98b2560c5240e38bee4f", "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543" } ] }