-
Notifications
You must be signed in to change notification settings - Fork 99
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
Migrate to aws sdk 2 for cloudwatch/aws cloud provider #730
Conversation
|
||
p.logger.WithField("ips", IP).Debug("Looking up instances") | ||
err := p.Ec2.DescribeInstancesPagesWithContext(ctx, input, func(page *ec2.DescribeInstancesOutput, lastPage bool) bool { | ||
hasPagesRemaining := true | ||
for hasPagesRemaining { | ||
pages++ |
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 know this is unchanged but just wondering if it should count a page if the describe failed later
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.
Fair question, don't have a strong opinion tbh
…unneeded Ec2Client type
@akavatl should we move to ready for review now? |
@irisgve Yes I will, I want to do some deeper testing but it shouldn't block this being reviewed(just being merged) |
Makefile
Outdated
@@ -66,14 +66,14 @@ build-gostatsd-fips: | |||
build-lambda-fips: | |||
@$(MAKE) build PKG=$(LAMBDA_PKG) BINARY_NAME="lambda-extension" GOBUILD_OPTIONAL_FLAGS="-tags fips" | |||
|
|||
build-all: pb/gostatsd.pb.go tools |
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 should be fine to leave here since build-all
covers building the proto
15edf35
to
383f932
Compare
No description provided.