Add AWS module_utils function to be able to check exceptions for a spec… #41202
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ansibot
added
WIP
|
The test |
jborean93
removed
the
needs_triage
ansibot
added
aws
cloud
module
|
The test The test |
|
thanks @ryansb, looks good |
s-hertel
changed the title
s-hertel
changed the title
ansibot
added
core_review
|
The test |
ansibot
added
needs_revision
willthames
approved these changes
Jun 7, 2018
|
The duplicate-exception test doesn't seem to like this change :( |
willthames
force-pushed
the
AnsibleAWSModule-error-code
branch
from
3434a01
to
f9228a1
Compare
|
@s-hertel I rebased your branch on latest devel and applied the same change to aws_eks_cluster so that if you work out how to appease the tests, aws_eks_cluster will get the fix too |
|
The test |
|
This |
ansibot
added
test
mattclay
reviewed
Jun 8, 2018
test/sanity/pylint/ignore.txt
Outdated
| lib/ansible/modules/cloud/amazon/ecs_ecr.py ansible-format-automatic-specification | ||
| lib/ansible/modules/cloud/amazon/rds_instance_facts.py duplicate-except | ||
| lib/ansible/modules/cloud/amazon/rds_snapshot_facts.py duplicate-except |
There was a problem hiding this comment.
Because the exception being caught is being dynamically generated and may or may not be the same class caught below. Pylint doesn't differentiate between this and static duplicates yet. I've opened a ticket in hopes that we can ignore only dynamically generate exception possible-duplicates in the future pylint-dev/pylint#2174 (so we don't accidentally mask static duplicates for which there is no reason).
There was a problem hiding this comment.
Why not add the error code check to the existing ClientError exception handlers instead of using dynamic types?
It looks like ClientError is already handled in most cases, so you'd end up with something like:
except botocore.exceptions.ClientError as e:
if is_boto3_error_code('NoSuchConfigRuleException'):
# do stuff here
else:
# do other stuff hereThere was a problem hiding this comment.
Yep, it just simplifies things and gives us the equivalent to what we thought from_code was (interestingly, from_code is also catching duplicate exceptions, pylint isn't catching it somehow).
Allows:
except is_boto3_error_code('ExceptionCodeWithSpecialHandling'):
pass
except (BotoCoreError, ClientError) as e:
module.fail_json_aws(e, msg="error message")
rather than
except ClientError as e:
if is_boto3_error_code('NoSuchConfigRuleException'):
pass
else:
module.fail_json_aws(e, msg="error message")
except BotoCoreError as e:
module.fail_json_aws(e, msg="error message")
There was a problem hiding this comment.
We should probably use inline pylint ignore comments for these instead of ignore.txt, since these are not issues that need fixing.
There was a problem hiding this comment.
Is the correct ignore the # noqa comment on the line? Or is there an ignore where you can specify the error?
There was a problem hiding this comment.
it's # pylint: disable=duplicate-except
ansibot
added
core_review
…arisons in Python 3+
ryansb
force-pushed
the
AnsibleAWSModule-error-code
branch
from
bc5facd
to
014f10b
Compare
ansibot
added
needs_maintainer
jacum
pushed a commit
to jacum/ansible
that referenced
this pull request
Jun 26, 2018
…ble#41202) * Add aws/core.py function to check for specific AWS error codes * Use sys.exc_info to get exception object if it isn't passed in * Allow catching exceptions with is_boto3_error_code * Replace from_code with is_boto3_error_code * Return a type that will never be raised to support stricter type comparisons in Python 3+ * Use is_boto3_error_code in aws_eks_cluster * Add duplicate-except to ignores when using is_boto3_error_code * Add is_boto3_error_code to module development guideline docs
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ific error code
WIP need to add this to the guidelines doc.
SUMMARY
Implementation for how we are using client.exceptions.from_code. from_code() is masking ClientErrors or ClientError subclasses.
ISSUE TYPE
COMPONENT NAME
lib/ansible/module_utils/aws/core.py
ANSIBLE VERSION