Update the webhook resource to be up to date #268
Conversation
- Adds support for updating the webhook resource. - Adds support to using retry functionality with configurable sleep/timeout.
|
Straight forward implementation of the changes. Found the main reason for |
docs/resources/webhook.md
Outdated
| <details> | ||
| <summary> | ||
| <b> | ||
| <i>Enable webhook after [v1.30.0](https://github.com/cloudamqp/terraform-provider-cloudamqp/releases/tag/v1.30.0)</i> |
There was a problem hiding this comment.
| <i>Enable webhook after [v1.30.0](https://github.com/cloudamqp/terraform-provider-cloudamqp/releases/tag/v1.30.0)</i> | |
| <i>Enable webhook after (v1.30.0)[https://github.com/cloudamqp/terraform-provider-cloudamqp/releases/tag/v1.30.0]</i> |
found by looking at the rendered version at https://github.com/cloudamqp/terraform-provider-cloudamqp/blob/webhook-retries/docs/resources/webhook.md
There was a problem hiding this comment.
Looks rather how Github interpret markdown different from https://registry.terraform.io/tools/doc-preview when used with html tags (noticed in other cases too). Both support the common syntax [text](link) in free text.
When using link between html tags:
- doc-preview wants
[text](link)between works,(text)[link]doesn't. - Github-preview get the same result between
[text](link),(text)[link]. non of them render correct.
Another example:
https://github.com/cloudamqp/terraform-provider-cloudamqp/blob/main/docs/resources/alarm.md
https://registry.terraform.io/providers/cloudamqp/cloudamqp/latest/docs/resources/alarm
Perhaps just remove the link or make the reference somewhere else.
Edit: Markdown kicked in here for my examples. Culprit seems to be <summary> tag.
<details><summary><i>[text](link)</i></summary></details> gives
[text](link)
There was a problem hiding this comment.
Oh
You can make the link with HTML then, does it work for both? GitHub should allow it at least
docs/resources/webhook.md
Outdated
| <details> | ||
| <summary> | ||
| <b> | ||
| <i>Enable webhook before v1.30.0</i> | ||
| </b> | ||
| </summary> | ||
|
|
||
| Doesn't support updating the resource, makes all argument using `ForceNew` behaviour. | ||
| Any changes to an argument will destroy and re-create the resource. Also no longer support | ||
| for using `retry_interval` argument in the backend, even though it's set to be required. | ||
|
|
There was a problem hiding this comment.
I wonder if we need to keep documentation for older versions in main? Maybe this is a special case as it involves ForceNew and the "warning" is warranted.
With that said, I think the after v1.30.0 example should go first, then the example for the old versions.
There was a problem hiding this comment.
Yeah, would be good to keep the information for the older version. Will play around and see what can be done. One thought is to create a separate page for the older person and provide a link it from the new webhook page. This will keep the main page clean from notes and "ifs". Especially with these behaviour changes.
There was a problem hiding this comment.
I think we can keep it here instead of creating a new page. I think it is more risk we forget about that page.
We can remove the info about the older version after a while/some number of releases.
There was a problem hiding this comment.
True will also create reference in registry. Created a separated section last in the page as a test, to keep the information as clean as possible. 33e4555
There was a problem hiding this comment.
docs/resources/webhook.md
Outdated
|
|
||
| ___ | ||
|
|
||
| * `retry_interval` - ~~(Required)~~ No longer supported in the backend and removed as an argument from v1.30.0. Still required to be set before v1.30.0. |
There was a problem hiding this comment.
I like this better than the <hr> line :)
| ___ | |
| * `retry_interval` - ~~(Required)~~ No longer supported in the backend and removed as an argument from v1.30.0. Still required to be set before v1.30.0. | |
| * ~~`retry_interval`~~ - ~~(Required)~~ No longer supported in the backend and removed as an argument from v1.30.0. Still required to be set before v1.30.0. |
There was a problem hiding this comment.
Verified it works with https://registry.terraform.io/tools/doc-preview
### WHY are these changes introduced? Underlying handling of webhooks relies on RPC request. Follow similar pattern as other endpoints and use retry functionality. ### WHAT is this pull request doing? Adds retry functionality to CRUD endpoints. ### HOW can this pull request be tested? Test together with cloudamqp/terraform-provider-cloudamqp#268
### WHY are these changes introduced? Underlying handling of webhooks relies on RPC request. Follow similar pattern as other endpoints and use retry functionality. ### WHAT is this pull request doing? Adds retry functionality to CRUD endpoints. ### HOW can this pull request be tested? Test together with #268
### WHY are these changes introduced? Underlying handling of webhooks relies on RPC request. Follow similar pattern as other endpoints and use retry functionality. ### WHAT is this pull request doing? Adds retry functionality to CRUD endpoints. ### HOW can this pull request be tested? Test together with #268
### WHY are these changes introduced? Underlying handling of webhooks relies on RPC request. Follow similar pattern as other endpoints and use retry functionality. ### WHAT is this pull request doing? Adds retry functionality to CRUD endpoints. ### HOW can this pull request be tested? Test together with #268
### WHY are these changes introduced? Underlying handling of webhooks relies on RPC request. Follow similar pattern as other endpoints and use retry functionality. ### WHAT is this pull request doing? Adds retry functionality to CRUD endpoints. ### HOW can this pull request be tested? Test together with #268
WHY are these changes introduced?
Updates have been made in the backend of how handling of webhooks are done.
It now supports updating an already created webhook and the client library supports using retry functionality.
WHAT is this pull request doing?
retry_intreval, no longer supported.HOW can this pull request be tested?
Test together with client library 84codes/go-api#50. Can be added to go-vcr-testing branch and re-recorded fixture cassette.