Add support for multiple HTTP methods, assertions, and request body in controller #58
Conversation
| { | ||
| Source: checkly.StatusCode, | ||
| Comparison: checkly.Equals, | ||
| Target: "200", |
There was a problem hiding this comment.
I would change this to apiCheck.SuccessCode, and revert its removal for backwards compat reasons. It should be marked deprecated though.
There was a problem hiding this comment.
If that is done, then shouldFail should also be updated.
| @@ -26,10 +26,14 @@ Any `metadata.labels` specified will be transformed into tags, for example `envi | |||
| |--------------|-----------|------------| | |||
| | `endpoint` | String; Endpoint to run the check against | none (*required) | | |||
| | `success` | String; The expected success code | none (*required) | | |||
There was a problem hiding this comment.
I believe this would no longer be required. However it should be kept for B/C and marked deprecated.
|
This looks very cool, I added a few comments and hopefully @akosveres is able to provide his opinion. Note that my comments are mostly about preserving backwards compatibility. We can mark things deprecated or potentially even hide them, but I do not want to outright break existing setups. If we tend to break things, people may choose to not update until absolutely necessary, which creates support issues for us in the long term. |
Issue ref.: #54
Description:
This PR extends the controller to support more than just the GET HTTP method. Key changes include:
Work In Progress
I believe the success parameter should be fully replaced by assertions. For example, the default assertion could check that statusCode == 200. However, I am uncertain how assertions would integrate with annotations, and further discussion might be required.
Why This PR?
While the broader changes are not yet finalized, this PR solves the immediate need on our end. I figured I would share it.