New Resource: aws_ses_template #2003
Conversation
|
Hey @bflad Can you fix the conflicts so that we can test & merge accordingly please. Thank you! 😄 🚀 |
| } | ||
|
|
||
| func TestAccAWSSesTemplate_Update(t *testing.T) { | ||
| t.Skipf("Skip due to SES.UpdateTemplate eventual consistency issues") |
There was a problem hiding this comment.
Can you please explain the effect of eventual consistency? We have many tests which can deal with this sort of situation, usually by retrying operations on a particular error code.
There was a problem hiding this comment.
The behavior I was seeing was that the AWS API always immediately returns a 200 for the SDK UpdateTemplate function, however when reading the resource the updates are not immediately propagated.
For example, changing the html field from "foo" to "bar" will mostly fail testing saying its still "foo" => "bar" for a few seconds. Sometimes it would pass on immediate read then fail on the acceptance testing framework's next refresh. After some time, the resource will eventually synchronize in AWS.
If you have some ideas for how to handle this situation, would love to know. Thanks.
There was a problem hiding this comment.
I see. We don't have any good helpers for this yet, but I observed very similar behaviour in a few other existing tests, so we should probably create one at some point.
aws/resource_aws_ses_template.go
Outdated
| Create: schema.DefaultTimeout(30 * time.Minute), | ||
| Update: schema.DefaultTimeout(30 * time.Minute), | ||
| Delete: schema.DefaultTimeout(30 * time.Minute), | ||
| }, |
There was a problem hiding this comment.
I don't see these timeouts being used anywhere 🤔
aws/resource_aws_ses_template.go
Outdated
| "id": { | ||
| Type: schema.TypeString, | ||
| Computed: true, | ||
| }, |
There was a problem hiding this comment.
This field is unnecessary, every resource gets one automatically. In fact if you rebased to the latest master you'd see a failing test because of that as we recently improved the schema validation.
| "text": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| ValidateFunc: validateSesTemplateText, |
There was a problem hiding this comment.
Thanks for all the validation here 👍
aws/resource_aws_ses_template.go
Outdated
| log.Printf("[DEBUG] Reading SES template: %#v", input) | ||
| gto, err := conn.GetTemplate(&input) | ||
| if err != nil { | ||
| if scErr, ok := err.(awserr.Error); ok && scErr.Code() == "TemplateDoesNotExist" { |
There was a problem hiding this comment.
We have a nice helper function for this which can save us some characters + 1 import line.
if isAWSErr(err, "TemplateDoesNotExist", "") {
radeksimko
added
the
waiting-response
|
Working on this now! |
* remove extraneous id field in schema * remove extraneous schema timeout definitions * use isAWSErr instead of manual awserr check
|
Merged master into PR and addressed:
If you have ideas for the |
radeksimko
removed
the
waiting-response
|
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! |
ghost
locked and limited conversation to collaborators
Supports all SES template CRUD activity however the UpdateTemplate call is eventually consistent and I couldn't find an easy workaround in the testing, so set those acceptance tests to skip for now.
This closes #1921 and #1981.