Skip to content
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

add support for setting minimum requested units in limit increase policy #2371

Merged
merged 5 commits into from May 10, 2018
Merged

Conversation

aardvarq
Copy link
Contributor

└─<cwb>$ git diff --minimal -U0  master
diff --git a/c7n/resources/account.py b/c7n/resources/account.py
index e71d9d81..a88df652 100644
--- a/c7n/resources/account.py
+++ b/c7n/resources/account.py
@@ -498,0 +499 @@ class RequestLimitIncrease(BaseAction):
+            'minimum-increase': {'type': 'number', 'minimum': 1},
@@ -534,0 +536 @@ class RequestLimitIncrease(BaseAction):
+        minimum_increase = self.data.get('minimum-increase')
@@ -540 +542 @@ class RequestLimitIncrease(BaseAction):
-                increase_by = max(increase_by, 1)
+                increase_by = max(increase_by, minimum_increase)

@@ -532,12 +533,15 @@ def process(self, resources):
limit_exceeded = resources[0].get('c7n:ServiceLimitsExceeded', [])
percent_increase = self.data.get('percent-increase')
amount_increase = self.data.get('amount-increase')

minimum_increase = self.data.get('minimum-increase')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than doing a check for if minimum-increase is None, you can just specify the default in the .get function itself, i.e. self.data.get('minimum-increase', 1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aha, indeed. Also, spelling counts, apparently ;-)

@aardvarq
Copy link
Contributor Author

1st commit - omitted default value. Fixed
2nd commit - typo'd "minimum". Fixed.

@aardvarq
Copy link
Contributor Author

@thisisshi

\o/

WARNING:test command found but not installed in testenv

@thisisshi
Copy link
Member

thisisshi commented May 10, 2018

@aardvarq the error is a linting one:

flake8 c7n tools/c7n_org tools/c7n_gcp tools/c7n_logexporter tools/c7n_mailer tools/c7n_sentry tools/c7n_sphinxext tools/zerodark tools/ops tools/c7n_azure
c7n/resources/account.py:537:1: W293 blank line contains whitespace
make: *** [lint] Error 1
ERROR: InvocationError for command '/usr/bin/make lint' (exited with code 2)

just remove the whitespace or line :)

@aardvarq
Copy link
Contributor Author

@aardvarq gently removes the somewhat liberal pylint alias from his shell profile.

@kapilt
Copy link
Collaborator

kapilt commented May 10, 2018

@aardvarq lgtm thanks for the pr, could you sign our contributor license agreement, referenced from the bottom of the project readme, direct link https://docs.google.com/forms/d/19LpBBjykHPox18vrZvBbZUcK6gQTj7qv1O5hCduAZFU/viewform

@aardvarq
Copy link
Contributor Author

done. Hope to file many more in the relatively near future. Thanks for the amazing work.

@kapilt kapilt merged commit b3e8264 into cloud-custodian:master May 10, 2018
@aardvarq aardvarq deleted the cwb branch May 10, 2018 22:18
lamyanba pushed a commit to lamyanba/cloud-custodian that referenced this pull request Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants