-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
New Resource: aws_quicksight_refresh_schedule
#30841
Conversation
Community NoteVoting for Prioritization
For Submitters
|
5b7c943
to
5dbd369
Compare
@jar-b - after further reading of the API docs I've realised that refresh schedules can be created from data sets which do not have refresh properties defined. The only requirement is that they must be of Accordingly, I've reworked the acceptance tests in this PR to use QuickSight S3 data sources, as is done in the As such this PR is ready to review and land! 👌 |
5e98179
to
b02a9b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks in great shape, and really impressive to see a Plugin-Framework based resource given we haven't published any contributor documentation yet!
I do have one general suggestion, which is to align the schema a bit more closely with the API:
resource "aws_quicksight_refresh_schedule" "example" {
# This is returned in both the root and inside the "RefreshSchedule" field of the
# DescribeRefreshSchedule API. To align with most other resources, this can stay
# at the schema root.
arn =
# Because this is required for delete calls (and the current Terraform ID), this could
# reasonably move into the root schema, despite technically being sent inside the
# schedule object for both create and update API calls.
schedule_id =
aws_account_id =
data_set_id =
schedule {
refresh_type =
# remove from here if moving to root
schedule_id =
start_after_date_time =
schedule_frequency {
interval =
time_of_the_day =
timezone =
refresh_on_day {
day_of_month =
day_of_week =
}
}
}
}
The justification behind this is explained in the HashiCorp plugin best practices documentation, but specific to the AWS provider this design allows practitioners to swap more seamlessly between other tools (CLI, SDKs, etc.) and the Terraform provider.
In this case, the APIs do have some strange characteristics - such as Create using a Schedule
field, while Describe calls it RefreshSchedule
, or Arn
being returned in two different levels of the Describe response - but moving most of the attributes inside a root schedule
object is slightly more consistent with the AWS APIs.
@jar-b - thanks for the review. Will make those changes now. |
3cf4d39
to
1cf338a
Compare
@jar-b - that should be those changes all ready. I went with the following resource structure in the end: resource "aws_quicksight_refresh_schedule" "schedule" {
arn = "..."
aws_account_id = "0123456789"
data_set_id = "id"
schedule_id = "example"
schedule {
refresh_type = "FULL_REFRESH"
start_after_date_time = "2023-12-18T00:00:00",
schedule_frequency {
interval = "WEEKLY"
time_of_the_day = "00:00"
timezone = "Europe/London"
refresh_on_day {
day_of_month = "1"
day_of_week = "MONDAY"
}
}
}
} Not exactly sure how I managed to misread the API structure so badly in the first place! |
1cf338a
to
e80d508
Compare
e80d508
to
92b4e9e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
$ make testacc PKG=quicksight TESTS=TestAccQuickSightRefreshSchedule_
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/quicksight/... -v -count 1 -parallel 20 -run='TestAccQuickSightRefreshSchedule_' -timeout 180m
=== RUN TestAccQuickSightRefreshSchedule_basic
=== PAUSE TestAccQuickSightRefreshSchedule_basic
=== RUN TestAccQuickSightRefreshSchedule_disappears
=== PAUSE TestAccQuickSightRefreshSchedule_disappears
=== RUN TestAccQuickSightRefreshSchedule_weeklyRefresh
=== PAUSE TestAccQuickSightRefreshSchedule_weeklyRefresh
=== RUN TestAccQuickSightRefreshSchedule_monthlyRefresh
=== PAUSE TestAccQuickSightRefreshSchedule_monthlyRefresh
=== CONT TestAccQuickSightRefreshSchedule_basic
=== CONT TestAccQuickSightRefreshSchedule_weeklyRefresh
=== CONT TestAccQuickSightRefreshSchedule_disappears
=== CONT TestAccQuickSightRefreshSchedule_monthlyRefresh
--- PASS: TestAccQuickSightRefreshSchedule_disappears (29.51s)
--- PASS: TestAccQuickSightRefreshSchedule_weeklyRefresh (31.97s)
--- PASS: TestAccQuickSightRefreshSchedule_monthlyRefresh (32.03s)
--- PASS: TestAccQuickSightRefreshSchedule_basic (32.21s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/quicksight 35.375s
Thanks for your contribution, @g-dx! 🥇 Really well done! |
This functionality has been released in v4.65.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
This PR adds a new
aws_quicksight_refresh_schedule
resource, allowing practitioners to manage scheduled refreshes of Quicksight data sets via Terraform.Relations
Closes #30788
Relates To #10990 & #30744
References
Output from Acceptance Testing