Feature/add data to webhook #474
Conversation
om-henners
force-pushed
the
feature/add_data_to_webhook
branch
from
e883baf
to
20214e6
Compare
|
@om-henners also thanks here! Makes sense. Yes, please add tests. There are existing notification/webhook tests: |
|
@tomkralidis you have time to review/maybe merge? |
This does increase the message size, though I imagine that's marginal, and is backwards compatible in that old keys remain in place and aren't modified. The goal is to make the actual GHC app more set and forget, and we can handle the errors with a little more discretion on the webhook end. The report is being sent as URLEncoded form data, which if we send a hierarchy just sends a single key. We want the entire report so encode the JSON as a string and send that. The keys are sorted to make testing easier.
Python3 division is always float. Much easier to check if the code is 400 or more.
Also update the external links to be https (as otherwise we have to wait for a bunch of redirects in the tests as well).
This uses the responses library to intercept the request so we can confirm that the body content is exactly as expected.
Makes sure we're not accidentally doubling up somewhere
om-henners
force-pushed
the
feature/add_data_to_webhook
branch
from
20214e6
to
48ff5f7
Compare
|
@justb4 added a simple test (sorry it took a little time to get to it my end). Happy to add more detail. But the most important thing I should note is that in doing so I introduced the https://github.com/getsentry/responses library as a testing lib to handle testing calling the webhook endpoint. (I did try speeding things up for the |
Add the run data to the webhook message.
The gaol is that we should be able to use the data coming to the webhook to triage and send to other systems (alerting systems etc), and to do that it's useful to distinguish between the error messages that come through. So this adds that data in without modifying the existing data.
Happy to add tests as required.