Add headers to webhook alerts #320
Conversation
…-sdk into webhooks-auth
There was a problem hiding this comment.
Could you double check that the generated files are up to date? Also, it's worth considering modeling the data types for the headers. Everything else about the webhooks is modeled pretty well, and without looking at the zuuul side I believe all you would need to do is nest a header serializer inside the webhood serializer.
| @@ -1242,6 +1242,8 @@ components: | |||
| properties: | |||
| template: | |||
| type: string | |||
| headers: | |||
There was a problem hiding this comment.
Is there no openapi type for the headers? In the client I see that the headers are expected to be a Optional[Dict[str, str]]. Should we model that here?
There was a problem hiding this comment.
That's a good point -- I'll make the cloud service changes so that makes it into the spec.
generated/model.py
Outdated
| @@ -251,7 +253,7 @@ class MultiClassificationResult(BaseModel): | |||
| class TextRecognitionResult(BaseModel): | |||
| confidence: Optional[confloat(ge=0.0, le=1.0)] = None | |||
| source: Optional[Source] = None | |||
| text: Optional[str] = Field(...) | |||
| text: str | |||
There was a problem hiding this comment.
Can you double check how this got changed? I'm not seeing a corresponding change in the public-api.yaml file. My expectation is that this line should mean that this is still optional. Is it possible that you generated the file with a work in progress yaml spec?
There was a problem hiding this comment.
Not sure how it got changed, but regenerating seems to have solved it.
generated/.openapi-generator/FILES
Outdated
| @@ -29,6 +29,11 @@ docs/ImageQueriesApi.md | |||
| docs/ImageQuery.md | |||
| docs/ImageQueryTypeEnum.md | |||
| docs/InlineResponse200.md | |||
| docs/InlineResponse2001.md | |||
There was a problem hiding this comment.
This is unrelated to my change -- not sure if I should keep those changes out of the public-api.yaml file when generating?
There was a problem hiding this comment.
This is related to #319. You can either ignore it here, or you can let me merge the other PR first and then you can merge main into your PR to get a cleaner diff
This PR allows the user to customize the headers field for their webhook alerts in addition to the payload. This will enable auth, which is required by a lot of different platforms.