-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix/m policies #447
Fix/m policies #447
Conversation
This reverts commit b1e7e9f.
@@ -313,6 +327,14 @@ def deploy_module_stack( | |||
|
|||
_logger.debug("project_managed_policy_output is : %s", stack_outputs) | |||
if project_managed_policy_stack_exists: | |||
if stack_outputs.get("StackStatus") and "_IN_PROGRESS" in stack_outputs.get("StackStatus"): | |||
_logger.info("The managed policy stack is not complete, waiting 60 seconds") | |||
time.sleep(60) |
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.
as discussed offline, its good to have this refactored with retry logic, while sleeping for 30 seconds
else: | ||
_logger.debug("project_managed_policy_output is : %s", stack_outputs) | ||
project_managed_policy_arn = stack_outputs.get("ProjectPolicyARN", None) | ||
retries = -1 |
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.
why should it retry once the project_managed_policy_arn
is obtained? shoudn't this be break
out of the while loop?
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.
it is not...it is setting the retries value to -1
retries = -1 | ||
else: | ||
_logger.debug("project_managed_policy_output does not exist") | ||
retries = -1 |
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.
Need for this retry?
we should retry only when the managed policy stack is in "*IN_PROGRESS" state
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.
this signals to break out of the loop... sets retries equal to -1
Issue #, if available:
#445
Description of changes:
Only try to delete the managed policy of no roles are attached
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.