Version 1.10.0 #30
Version 1.10.0 #30
Conversation
set max results for describing EC2 instances to 1000
|
This PR just in case if somebody also needs feature to display NOTDEF for check if limit in AWS is not default (was increased) |
|
Hey @alt-dima, thanks for your work! |
|
Ok! No problem:) |
|
@alt-dima No need to close the PR. You can just revert your changes, push to the same branch and the PR will update. |
| @property | ||
| def maximum(self): | ||
| # return self.boto_session.client('iam').get_account_summary()['SummaryMap']['UsersQuota'] | ||
| return int(-1) |
There was a problem hiding this comment.
Why should the maximum value be -1?
There was a problem hiding this comment.
maximum -1 means, that getting maximum value from AWS is not working. I have got an error NoSuchResourceException in this specific check.
There was a problem hiding this comment.
Check my comment here. I came across the same exception and would like to fall back to the default value (1000) in this case.
There was a problem hiding this comment.
Yes, i saw it, but decided to specify -1 like indicator for user, that this value was got not from AWS, but hardcoded (and user should check it manually).
I changed the same logic in many places and with | grep -e NOTDEF -e /-1 I can get an indicators, that should be checked.
This is just my opinion and specific needs in my task.
There was a problem hiding this comment.
Not sure if you got that correctly. At the moment there are no hardcoded values for any checks. If there is no dedicated maxmium function defined the check will default to this function cause all checks are subclassing QuotaCheck.
There was a problem hiding this comment.
I still don't understand what you mean. We never "fail" getting the maximum from AWS. Anyway I'm pretty sure people will be confused when seeing this output 0/-1/15 cause it's very hard to understand. If you need this kind of output feel free to use your own fork.
There was a problem hiding this comment.
I changed this logic too. To see, where the maximum value is not from AWS (was "copied" from "default"). In the future this should be fixed by AWS or ourself to get correct max/def value from AWS.
If we can't get maximum or default value from AWS, then we should inform about that. And also we can be sure, that we are not over-quota. For example, this is very important with number of S3 buckets. You can't check a maximum value from AWS (only default and current). Maximum can tell only support. So, if you see it, you should check it mannually:
S3 Buckets per account [16634349525/us-east-1]: 209/-1/100 X
or another example:
ERROR get maximum : appmesh / L-AC861A39
App Meshes per account [1668413434345]: 0/-1/15 !
There was a problem hiding this comment.
Think we just have a different expectation of what this tool should do. Feel free to implement this in your fork. I'm not going to merge this logic into this project.
Added