Custom payloads for webhook alerts #314
Conversation
…ight/python-sdk into webhooks-custom-payloads
brandon-wada
left a comment
brandon-wada
left a comment
There was a problem hiding this comment.
Rambled a bunch in my comments, but things look good as is
| payload_template=payload_template, | ||
| ) | ||
|
|
||
| def make_payload_template(self, template: str) -> PayloadTemplate: |
There was a problem hiding this comment.
I've left bad example code for you, but I appreciate that you followed the pattern
We shouldn't lean too hard into the make_* format. Most of the classes exist and it shouldn't be strange for a user to construct a payload as PayloadTemplate(template='mytr'). These generated classes are what we return via our custom SDK functions, so they shouldn't be strange to users. We do have make_* commands for alerts (specifically actions and conditions) because they have these horribly undiscoverable set of parameters, and the make_* commands give a convenient docstring for discovering those parameters.
The real fix is to make those parameters well defined and discoverable, but that's way down my priority stack.
This is all my ramblings, I don't actually have any requests from this comment, just that you know the background
|
|
||
| def make_webhook_action(self, url: str, include_image: bool) -> WebhookAction: | ||
| def make_webhook_action( | ||
| self, url: str, include_image: bool, payload_template: Optional[PayloadTemplate] = None |
There was a problem hiding this comment.
nit, if reasonable I think the payload_template should be allowed to be a string here as well.
There was a problem hiding this comment.
I'd prefer not to make that change -- currently payload_template only has one text field, but I think we might be adding more soon. Specifically, we might be adding a separate header template for auth or a field indicating what kind of payload template it is (we currently only support json).
This PR adds the ability to provide custom payloads for webhook alerts. It will be included in the 0.22.0 SDK release.