56 lines
2.9 KiB
Plaintext
56 lines
2.9 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "910bf227_33eb0bf4",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 3
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 33536
|
|
},
|
|
"writtenOn": "2022-10-31T13:23:58Z",
|
|
"side": 1,
|
|
"message": "Thanks Facundo for your review.\n\nI\u0027ve changed this patch to have a single configuration named \"haproxy-check-http-config\" as you pointed in the review.\n\nCould you also review the change on charm-layer-openstack that is dependent on this change?\n\nhttps://review.opendev.org/c/openstack/charm-layer-openstack/+/862752\n\nThanks",
|
|
"revId": "b2d379e6d95c61e9fb167067bbc7072dddc5d4f2",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "1c85cf86_dab07a89",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 3
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 31836
|
|
},
|
|
"writtenOn": "2022-11-03T14:38:01Z",
|
|
"side": 1,
|
|
"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"
|
|
}
|
|
],
|
|
"submitRequirementResults": []
|
|
} |