-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
add helpers #37
add helpers #37
Conversation
@josh-padnick @brikis98 please review and let me know if you have comments |
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.
This is a great addition, thx!
aws/sqs.go
Outdated
|
||
// Waits to receive a message from on the queueUrl. Since the API only allows us to wait a max 20 seconds for a new | ||
// message to arrive, we must loop TIMEOUT/20 number of times to be able to wait for a total of TIMEOUT seconds | ||
func WaitForQueueMessage(awsRegion string, queueUrl string, timeout int) (string, string, error) { |
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.
It's not obvious what the two returned strings are. At a minimum, comment it. Better yet would be to wrap it in a struct with named params.
resources/base_resources.go
Outdated
func CreateBaseRandomResourceCollection(t *testing.T, requireNatGateway bool) *terratest.RandomResourceCollection { | ||
exludedRegions := REGIONS_WITHOUT_T2_NANO | ||
|
||
if (requireNatGateway) { |
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.
Hm, we also have a case in module-ecs where we exclude specific regions that don't support ECS. Perhaps other similar cases. I wonder if we shouldn't have some sort of bitmask or enum style flag to specify the types of resources you plan to create and the function automatically excludes the appropriate region?
} | ||
|
||
// As of 6/9/16, these AWS regions do not support t2.nano instances | ||
var REGIONS_WITHOUT_T2_NANO = []string{ |
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.
Ah, another option is to define these constants and for the CreateBaseRandomResourceCollection
to take in a splat (..
) to exclude, which can optionally be left blank.
No description provided.