-
Notifications
You must be signed in to change notification settings - Fork 695
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
Use filter query when describing instances #356
Conversation
res0nance
commented
May 28, 2019
- Get the API to do the filtering instead of doing it ourselves
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.
Looks like a nice improvement so far.
@jvz I think I'm done with this just turning it into the equivalent call letting the aws api deal with as much of the filtering as possible |
Can you either add a test or describe how to manually test this? |
Behaviour doesn't change it has the exact behaviour as before as for automated testing we need a way to mock the aws api for that |
@thoulen Can you take a look at this? This should help reduce the amount of data we pull from aws api and offload some of the filtering logic back to aws. |
I am testing the patch |
Added a fix to take care of a possible miscount when nexttoken is available |
This PR also includes a bugfix where instance counting will be wrong if there are more than 1000 instances because we ignore the next token |
Tested this and my instance caps are still properly respected so I'm merging it. |