-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
resource/aws_appsync_resolver: Add CachingConfig support #12747
Conversation
The CI job has completed successfully but the status on github is still Pending... 🤔 https://travis-ci.org/github/terraform-providers/terraform-provider-aws/builds/673029687 |
365367d
to
5e25214
Compare
5e25214
to
6d1b740
Compare
Whew, this PR is ready for review! |
This PR closes #13230 |
"caching_config": { | ||
Type: schema.TypeList, | ||
Optional: true, | ||
Elem: &schema.Resource{ |
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.
Please add MaxItems: 1,
.
aws/resource_aws_appsync_resolver.go
Outdated
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"caching_keys": { | ||
Type: schema.TypeList, |
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.
I suggest
Type: schema.TypeSet,
Set: schema.HashString,
here as order is not important.
aws/resource_aws_appsync_resolver.go
Outdated
m := l[0].(map[string]interface{}) | ||
|
||
cachingConfig := &appsync.CachingConfig{ | ||
CachingKeys: expandStringList(m["caching_keys"].([]interface{})), |
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.
CachingKeys: expandStringSet(m["caching_keys"].(*schema.Set)),
aws/structure.go
Outdated
} | ||
|
||
m := map[string]interface{}{ | ||
"caching_keys": flattenStringList(c.CachingKeys), |
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.
flattenStringSet(c.CachingKeys)
aws/structure.go
Outdated
@@ -5565,6 +5565,23 @@ func flattenAppsyncPipelineConfig(c *appsync.PipelineConfig) []interface{} { | |||
return []interface{}{m} | |||
} | |||
|
|||
func flattenAppsyncCachingConfig(c *appsync.CachingConfig) []interface{} { |
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.
Maybe move to aws/resource_aws_appsync_resolver.go
to avoid overloading this file?
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.
That makes sense, thank you!
I'll also move flattenAppsyncPipelineConfig
together with flattenAppsyncCachingConfig
to aws/resource_aws_appsync_resolver.go
. 💡
Btw, besides flattenAppsync**
, I can see some resource-specific function as well in structure.go
but I'm not sure why the functions are in the structure.go
. 🤔
After the updates, I've checked the acceptance tests have passed. 😌 Output:
|
@ewbankkit Thank you for your review! Please have another look when you have time. 😃 |
Verified acceptance tests: $ make testacc TEST=./aws/ TESTARGS='-run=TestAccAwsAppsyncResolver_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAwsAppsyncResolver_ -timeout 120m
=== RUN TestAccAwsAppsyncResolver_basic
=== PAUSE TestAccAwsAppsyncResolver_basic
=== RUN TestAccAwsAppsyncResolver_disappears
=== PAUSE TestAccAwsAppsyncResolver_disappears
=== RUN TestAccAwsAppsyncResolver_DataSource
=== PAUSE TestAccAwsAppsyncResolver_DataSource
=== RUN TestAccAwsAppsyncResolver_RequestTemplate
=== PAUSE TestAccAwsAppsyncResolver_RequestTemplate
=== RUN TestAccAwsAppsyncResolver_ResponseTemplate
=== PAUSE TestAccAwsAppsyncResolver_ResponseTemplate
=== RUN TestAccAwsAppsyncResolver_multipleResolvers
=== PAUSE TestAccAwsAppsyncResolver_multipleResolvers
=== RUN TestAccAwsAppsyncResolver_PipelineConfig
=== PAUSE TestAccAwsAppsyncResolver_PipelineConfig
=== RUN TestAccAwsAppsyncResolver_CachingConfig
=== PAUSE TestAccAwsAppsyncResolver_CachingConfig
=== CONT TestAccAwsAppsyncResolver_basic
=== CONT TestAccAwsAppsyncResolver_multipleResolvers
=== CONT TestAccAwsAppsyncResolver_CachingConfig
=== CONT TestAccAwsAppsyncResolver_PipelineConfig
=== CONT TestAccAwsAppsyncResolver_RequestTemplate
=== CONT TestAccAwsAppsyncResolver_ResponseTemplate
=== CONT TestAccAwsAppsyncResolver_DataSource
=== CONT TestAccAwsAppsyncResolver_disappears
--- PASS: TestAccAwsAppsyncResolver_PipelineConfig (29.86s)
--- PASS: TestAccAwsAppsyncResolver_disappears (30.41s)
--- PASS: TestAccAwsAppsyncResolver_CachingConfig (37.19s)
--- PASS: TestAccAwsAppsyncResolver_multipleResolvers (41.31s)
--- PASS: TestAccAwsAppsyncResolver_ResponseTemplate (46.97s)
--- PASS: TestAccAwsAppsyncResolver_DataSource (50.75s)
--- PASS: TestAccAwsAppsyncResolver_RequestTemplate (50.87s)
--- PASS: TestAccAwsAppsyncResolver_basic (75.71s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 75.754s |
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.
Thanks @ackintosh 🚀
Output from acceptance testing:
--- PASS: TestAccAwsAppsyncResolver_disappears (17.59s)
--- PASS: TestAccAwsAppsyncResolver_CachingConfig (21.28s)
--- PASS: TestAccAwsAppsyncResolver_PipelineConfig (21.32s)
--- PASS: TestAccAwsAppsyncResolver_ResponseTemplate (27.75s)
--- PASS: TestAccAwsAppsyncResolver_basic (30.41s)
--- PASS: TestAccAwsAppsyncResolver_RequestTemplate (32.44s)
--- PASS: TestAccAwsAppsyncResolver_multipleResolvers (33.10s)
--- PASS: TestAccAwsAppsyncResolver_DataSource (35.67s)
This has been released in version 2.62.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 for triage. Thanks! |
…2747) Output from acceptance testing: ``` --- PASS: TestAccAwsAppsyncResolver_disappears (17.59s) --- PASS: TestAccAwsAppsyncResolver_CachingConfig (21.28s) --- PASS: TestAccAwsAppsyncResolver_PipelineConfig (21.32s) --- PASS: TestAccAwsAppsyncResolver_ResponseTemplate (27.75s) --- PASS: TestAccAwsAppsyncResolver_basic (30.41s) --- PASS: TestAccAwsAppsyncResolver_RequestTemplate (32.44s) --- PASS: TestAccAwsAppsyncResolver_multipleResolvers (33.10s) --- PASS: TestAccAwsAppsyncResolver_DataSource (35.67s) ```
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Community Note
Release note for CHANGELOG:
Output from acceptance testing: