-
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
issue #488 - Update ElasticCache default limits #491
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #491 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 40 40
Lines 2901 2901
Branches 441 441
=========================================
Hits 2901 2901
Continue to review full report at Codecov.
|
one of the things I've been unable to do is generate the updated documentation (to update https://github.com/jantman/awslimitchecker/blob/master/docs/source/limits.rst for example) - the command ``tox -e docs``` does not seem to catch the new defaults |
Codecov Report
@@ Coverage Diff @@
## develop #491 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 42 42
Lines 3005 3005
Branches 448 448
=========================================
Hits 3005 3005
Continue to review full report at Codecov.
|
@sebasrp It looks as though something may have changed again since this PR was merged. Looking at https://docs.aws.amazon.com/general/latest/gr/elasticache-service.html today I now see a "Nodes per cluster (Memcached)" limit with a default that's back down to 20, and a "Nodes per cluster per instance type (Redis cluster mode enabled)" limit with a default of 90. The current awslimitchecker code only appears to check Memcache clusters for "Nodes per Cluster". |
I am trying to figure out the latest info on the documentation is correct or not. |
Ok, thanks so much! I'm planning the 10.0.0 release on Monday December 7th. |
It seems elasticache is not onboarded in aws service quotas, so the information we have is purely from the documentation. I will update the PR to match the documentation and also reach out to one of the TAMs to confirm in parallel |
Ok, thanks so much! If you're reaching out to your TAMs, I'll hold off on opening a ticket on one of our accounts. For what it's worth, the ElastiCache docs are on github ( https://github.com/awsdocs/amazon-elasticache-docs )... but, very unfortunately, the service limits docs have been moved from the individual service docs to the General Reference (e.g. https://docs.aws.amazon.com/general/latest/gr/elasticache-service.html ) and that doc is not on github... so we no longer have commit history to see when the limit docs changed. |
I've updated the PR to revert to 20 nodes per cluster (memcached). |
Summary
issue #488 - Update the following ElasticCache default limits: Nodes, Nodes per Cluster, Subnet Groups, Parameter Groups
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.