Update patch set 3

Patch Set 3: Code-Review-1

(1 comment)

Patch-set: 3
Reviewer: Gerrit User 12549 <12549@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Code-Review=-1
Attention: {"person_ident":"Gerrit User 33536 \u003c33536@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_12549\u003e replied on the change"}

b2d379e6d95c61e9fb167067bbc7072dddc5d4f2

@@ -33,6 +33,23 @@
       "message": "Nice, thanks!",
       "revId": "b2d379e6d95c61e9fb167067bbc7072dddc5d4f2",
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
+    },
+    {
+      "unresolved": false,
+      "key": {
+        "uuid": "3218d9f8_07bcfed8",
+        "filename": "/PATCHSET_LEVEL",
+        "patchSetId": 3
+      },
+      "lineNbr": 0,
+      "author": {
+        "id": 12549
+      },
+      "writtenOn": "2022-11-08T11:44:56Z",
+      "side": 1,
+      "message": "This pr is targeted against reactive charms and is seems to provide an inconsistent approach with the corresponding classic charm change.\n\nhttps://review.opendev.org/c/openstack/charm-glance/+/862521\n\nI think the classic charm approach of not exposing new charm config options feels like the right approach as the charm should know how to check the health of the payload.\n\nIf you decide to follow the classic charm pattern I *think* you could have the charm class specify a dict like:\n\n```\nclass DesignateCharm(ch_plugins.PolicydOverridePlugin,\n                     openstack_charm.HAOpenStackCharm):\n    ....\n    healthcheck \u003d {\n        \u0027option\u0027: \u0027httpchk GET /healthcheck\u0027,\n        \u0027http-check\u0027: \u0027expect status 200\u0027,\n```\n\nThe APIConfigurationAdapter *1 has access to an instance of the charm class so it could read that dict and set it as a property which can then be referenced by the PeerHARelationAdapter via self.api_config_adapter.\n\n*1 https://opendev.org/openstack/charms.openstack/src/branch/master/charms_openstack/adapters.py#L726\n\nAs with the classic charm example I am not convinced this is going to work when the endpoints are using TLS.\n\nFor future reference I think this work would have benefited from a spec.",
+      "revId": "b2d379e6d95c61e9fb167067bbc7072dddc5d4f2",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
     }
   ]
 }