-
Notifications
You must be signed in to change notification settings - Fork 188
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
Replace "Max spot instance requests per region" limit with vCPU-based limits #547
Conversation
@TagadaPoe I'm going to try to get to these PRs today, sorry for the delay. Could you please update your PR to allow changes from maintainers? This needs to be rebased on develop, and the others will need to be rebased as I merge them in. Thanks! |
@TagadaPoe The code here looks good to me, but I'm a little confused by the default limits? In this PR I see the defaults as mostly 128 or 64, but at https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-spot-limits.html they seem to be much lower, i.e. 11 or 16 for many. Would you be so kind as to explain... I think I'm missing something here, or confused. |
I cannot see the "Allow edits from maintainers" link, is this because the PR is already in the "jantman/awslimitchecker repository" ? |
I fixed that, I am not sure where I took those values from... |
Codecov Report
@@ Coverage Diff @@
## develop #547 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 42 42
Lines 3030 3025 -5
Branches 455 453 -2
=========================================
- Hits 3030 3025 -5
Continue to review full report at Codecov.
|
… limits BREAKING CHANGE As mentioned in issue jantman#502, the limit "Max spot instance requests per region" was removed from AWS and replaced by the following limits, based on the number of vCPU: - All F Spot Instance Requests - All G Spot Instance Requests - All Inf Spot Instance Requests - All P Spot Instance Requests - All Standard (A, C, D, H, I, M, R, T, Z) Spot Instance Requests - All X Spot Instance Requests As a result, the usage reported by awslimitchecker is not relevant anymore. In order to fix this, this commit replaces the call to the EC2 API describe_spot_instance_requests() with a call to the CloudWatch API, since the usage of Spot requests vCPUs is now available as a metric in CloudWatch. In addition, the applied limits are now retrieved from the Service Quotas service.
Set default limits as specified in https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-spot-limits.html
bbeced3
to
cbe9040
Compare
Hello @jantman, as said by @TagadaPoe we couldn't find the option to allow you to rebase our branch. Anyway, I rebased all three branches, don't hesitate to ping me if you need rebasing or anything else. |
Thank you so much for this, and apologies for the delay in merging it! |
This has been released in 12.0.0, which is now live on PyPI and Docker Hub. Thank you so much, and apologies for the delay! |
Summary
BREAKING CHANGE
As mentioned in issue #502, the limit "Max spot instance requests per region" was removed from AWS and replaced by the following limits, based on the number of vCPU:
As a result, the usage reported by awslimitchecker is not relevant anymore.
In order to fix this, this commit replaces the call to the EC2 API describe_spot_instance_requests() with a call to the CloudWatch API, since the usage of Spot requests vCPUs is now available as a metric in CloudWatch.
In addition, the applied limits are now retrieved from the Service Quotas service.
Pull Request Checklist
writing tests for the code, that's fine, just mention that in the summary and either
ask for assistance, or clarify that you'd like someone else to handle the tests. PRs that
include complete test coverage will usually be merged faster.
tox -e docs
connect()
andconnect_resource()
methods, inherited fromawslimitchecker.connectable.Connectable
if you're working on a GitHub issue (i.e. "issue #x - "). Please
refrain from using the "fixes #x" notation unless you are sure that the
the issue is fixed in that commit.
Contributor License Agreement
By submitting this work for inclusion in awslimitchecker, I agree to the following terms:
is being made under the same license as the awslimitchecker project (the Affero GPL v3,
or any subsequent version of that license if adopted by awslimitchecker).
this pull request grants a perpetual, global, unlimited license for it to be used and distributed
under the terms of awslimitchecker's license.