Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Slo Correction] Add docs for recurring slo correction #1256

Merged
merged 10 commits into from
Nov 2, 2021

Conversation

iminoso
Copy link
Contributor

@iminoso iminoso commented Oct 28, 2021

Add documentation for recurring slo correction fields.

@iminoso iminoso marked this pull request as ready for review October 28, 2021 21:01
@iminoso iminoso requested a review from a team as a code owner October 28, 2021 21:01
resource "datadog_slo_correction" "example_slo_correction_with_recurrence" {
category = "Scheduled Maintenance"
description = "correction example with recurrence"
start = 1735707000
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit - I wonder if there is a way to add a one line comment with human readable form
e.g start = 1735707000 # Tue Dec 31 23:50:00 EST 2024

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is automatically generated so I'm not sure ..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc, it is generated from the tf file under examples/, you can modify there.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see, no worries then

resource "datadog_slo_correction" "example_slo_correction_with_recurrence" {
category = "Scheduled Maintenance"
description = "correction example with recurrence"
start = 1735707000
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

athap
athap previously approved these changes Oct 29, 2021
Copy link

@athap athap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a nit comment
Looks good otherwise!

@@ -36,7 +36,7 @@ func resourceDatadogSloCorrection() *schema.Resource {
"end": {
Type: schema.TypeInt,
Required: true,
Copy link
Contributor

@zhengshizhao zhengshizhao Oct 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required: true should be removed for rrule. may need to add a validation for one time correction if end is not given.

"rrule": {
Type: schema.TypeString,
Optional: true,
Description: "Recurrence rules as defined in the iCalendar RFC 5545.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a line for somefields that're not supported? something like dtstart, dtend, duration etc?

Copy link
Contributor Author

@iminoso iminoso Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah we don't explicitly throw errors if things like dtstart and dtend are actually included in the rrule string - I think we can add a follow up for that then include in this doc

@zhengshizhao
Copy link
Contributor

The provider doesn't seem to be updated for recurring correction. shouldn't we update the resource first before we publish the doc?

Required: true,
Description: "Ending time of the correction in epoch seconds.",
Optional: true,
Description: "Ending time of the correction in epoch seconds. Required for one time corrections, but optional if `rrule` is specified",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we mark this field to conflict with rrule and duration?

datadog/tests/resource_datadog_slo_correction_test.go Outdated Show resolved Hide resolved
@skarimo skarimo enabled auto-merge (squash) November 2, 2021 17:01
@skarimo
Copy link
Member

skarimo commented Nov 2, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@skarimo skarimo merged commit a169eb9 into master Nov 2, 2021
@skarimo skarimo deleted the iminoso/slo-corrections-rrule branch November 2, 2021 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants