r/cloudtrail: Retry if IAM role isn't propagated yet #1312
Conversation
radeksimko
force-pushed
the
b-cloudtrail-retry
branch
4 times, most recently
from
b5201eb to
f189508
Compare
aws/resource_aws_cloudtrail.go
Outdated
| if isAWSErr(err, "InvalidCloudWatchLogsRoleArnException", "Access denied.") { | ||
| return resource.RetryableError(err) | ||
| } | ||
| if isAWSErr(err, "InvalidCloudWatchLogsLogGroupArnException", "Access denied.") { |
There was a problem hiding this comment.
What do you think of having a single if, checking for both conditions?
There was a problem hiding this comment.
Also, would also use ErrCodeInvalidCloudWatchLogsLogGroupArnException here.
There was a problem hiding this comment.
I think the two if-s look tiny little bit cleaner (easier to read) compared to 1 if with 2 conditionals which would need to be line-wrapped (b/c long lines aren't easy to read either).
There was a problem hiding this comment.
Agreed about constant though. I'll change that.
aws/resource_aws_cloudtrail.go
Outdated
| var err error | ||
| t, err = conn.CreateTrail(&input) | ||
| if err != nil { | ||
| if isAWSErr(err, "InvalidCloudWatchLogsRoleArnException", "Access denied.") { |
There was a problem hiding this comment.
I would use the cloudtail.ErrCodeInvalidCloudWatchLogsRoleArnException constant, just for sanity and typo-less issues. Thoughts?
There was a problem hiding this comment.
It's 17 extra characters! 
but trolling aside - yes, constant makes sense in this context and it may help avoiding typos. I'll change it.
aws/resource_aws_cloudtrail.go
Outdated
| if isAWSErr(err, "InvalidCloudWatchLogsRoleArnException", "Access denied.") { | ||
| return resource.RetryableError(err) | ||
| } | ||
| if isAWSErr(err, "InvalidCloudWatchLogsLogGroupArnException", "Access denied.") { |
radeksimko
force-pushed
the
b-cloudtrail-retry
branch
from
f189508 to
0d7e9d8
Compare
|
@Ninir addressed your comments regarding constants. 😉 |
|
👍 |
|
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
Closes #693
The first error code (
InvalidCloudWatchLogsRoleArnException) comes from the bug report, second one (InvalidCloudWatchLogsLogGroupArnException) comes from testing of #1357.