Repository: ansible/ansible
Pull request: Improve cloudwatchevent_rule exception handling
##### SUMMARY

Fix broken exception handling and improve non-existent exception handling

This PR is designed as three commits. The first of them should be backported to 2.4, the other two are additional improvements for devel

Fixes #30806

##### ISSUE TYPE
 - Bugfix Pull Request

##### COMPONENT NAME
cloudwatchevent_rule

##### ANSIBLE VERSION
```
ansible 2.5.0 (devel dc8aedb274) last updated 2017/09/22 10:17:03 (GMT +1000)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/will/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/will/src/ansible/lib/ansible
  executable location = /home/will/src/ansible/bin/ansible
  python version = 2.7.13 (default, Sep  5 2017, 08:53:59) [GCC 7.1.1 20170622 (Red Hat 7.1.1-3)]

```


Problem statement:
Ansible 2.4.0 complains "NameError: global name 'botocore' is not defined" in cloudwatchevent_rule module
##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
AWS cloudwatchevent_rule

##### ANSIBLE VERSION
2.4.0.0

After installing 2.4, creating CloudWatch Rules had this error message. 
```
The full traceback is:
Traceback (most recent call last):
  File "/var/folders/2d/bq5mpnnn38b06hwfr4ck8q1w9nfwpk/T/ansible_Zao6N2/ansible_module_cloudwatchevent_rule.py", line 427, in <module>
    main()
  File "/var/folders/2d/bq5mpnnn38b06hwfr4ck8q1w9nfwpk/T/ansible_Zao6N2/ansible_module_cloudwatchevent_rule.py", line 415, in main
    cwe_rule_manager.ensure_present()
  File "/var/folders/2d/bq5mpnnn38b06hwfr4ck8q1w9nfwpk/T/ansible_Zao6N2/ansible_module_cloudwatchevent_rule.py", line 267, in ensure_present
    rule_description = self.rule.describe()
  File "/var/folders/2d/bq5mpnnn38b06hwfr4ck8q1w9nfwpk/T/ansible_Zao6N2/ansible_module_cloudwatchevent_rule.py", line 145, in describe
    except botocore.exceptions.ClientError as e:
NameError: global name 'botocore' is not defined

fatal: [localhost]: FAILED! => {
    "changed": false,
    "failed": true,
    "module_stderr": "Traceback (most recent call last):\n  File \"/var/folders/2d/bq5mpnnn38b06hwfr4ck8q1w9nfwpk/T/ansible_Zao6N2/ansible_module_cloudwatchevent_rule.py\", line 427, in <module>\n    main()\n  File \"/var/folders/2d/bq5mpnnn38b06hwfr4ck8q1w9nfwpk/T/ansible_Zao6N2/ansible_module_cloudwatchevent_rule.py\", line 415, in main\n    cwe_rule_manager.ensure_present()\n  File \"/var/folders/2d/bq5mpnnn38b06hwfr4ck8q1w9nfwpk/T/ansible_Zao6N2/ansible_module_cloudwatchevent_rule.py\", line 267, in ensure_present\n    rule_description = self.rule.describe()\n  File \"/var/folders/2d/bq5mpnnn38b06hwfr4ck8q1w9nfwpk/T/ansible_Zao6N2/ansible_module_cloudwatchevent_rule.py\", line 145, in describe\n    except botocore.exceptions.ClientError as e:\nNameError: global name 'botocore' is not defined\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE",
    "rc": 0
}
```
Tried Ansible 2.5.0, same result. 

Looking cloudwatchevent_rule.py, 

118 try:
119     import boto3.exception
120     import botocore.exceptions
121 except ImportError:
122     # module_utils.ec2.HAS_BOTO3 will do the right thing
123     pass


Line 119, is it boto3.exceptions (with s)?

Hints from issue discussion:
@fengchen8 Greetings! Thanks for taking the time to open this issue. In order for the community to handle your issue effectively, we need a bit more information. 

Here are the items we could not find in your description:
- issue type
- ansible version
- component name

Please set the description of this issue with this template:
https://raw.githubusercontent.com/ansible/ansible/devel/.github/ISSUE_TEMPLATE.md

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: issue_missing_data --->
cc @jsdalton
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: PEP8 tidy up

Patch to review:
diff --git a/lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py b/lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py
index cc2ae97f372b36..08d42d25bfb1b1 100644
--- a/lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py
+++ b/lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py
@@ -116,13 +116,12 @@
 '''  # NOQA
 
 try:
-    import boto3.exception
-    import botocore.exceptions
+    import botocore
 except ImportError:
     # module_utils.ec2.HAS_BOTO3 will do the right thing
     pass
 
-from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.aws.core import AnsibleAWSModule
 from ansible.module_utils.ec2 import (HAS_BOTO3, boto3_conn, camel_dict_to_snake_dict,
                                       ec2_argument_spec, get_aws_connection_info)
 
@@ -137,6 +136,7 @@ def __init__(self, module, name, client, schedule_expression=None,
         self.event_pattern = event_pattern
         self.description = description
         self.role_arn = role_arn
+        self.module = module
 
     def describe(self):
         """Returns the existing details of the rule in AWS"""
@@ -146,7 +146,9 @@ def describe(self):
             error_code = e.response.get('Error', {}).get('Code')
             if error_code == 'ResourceNotFoundException':
                 return {}
-            raise
+            self.module.fail_json_aws(e, msg="Could not describe rule %s" % self.name)
+        except botocore.exceptions.BotoCoreError as e:
+            self.module.fail_json_aws(e, msg="Could not describe rule %s" % self.name)
         return self._snakify(rule_info)
 
     def put(self, enabled=True):
@@ -163,26 +165,39 @@ def put(self, enabled=True):
             request['Description'] = self.description
         if self.role_arn:
             request['RoleArn'] = self.role_arn
-        response = self.client.put_rule(**request)
+        try:
+            response = self.client.put_rule(**request)
+        except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
+            self.module.fail_json_aws(e, msg="Could not create/update rule %s" % self.name)
         self.changed = True
         return response
 
     def delete(self):
         """Deletes the rule in AWS"""
         self.remove_all_targets()
-        response = self.client.delete_rule(Name=self.name)
+
+        try:
+            response = self.client.delete_rule(Name=self.name)
+        except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
+            self.module.fail_json_aws(e, msg="Could not delete rule %s" % self.name)
         self.changed = True
         return response
 
     def enable(self):
         """Enables the rule in AWS"""
-        response = self.client.enable_rule(Name=self.name)
+        try:
+            response = self.client.enable_rule(Name=self.name)
+        except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
+            self.module.fail_json_aws(e, msg="Could not enable rule %s" % self.name)
         self.changed = True
         return response
 
     def disable(self):
         """Disables the rule in AWS"""
-        response = self.client.disable_rule(Name=self.name)
+        try:
+            response = self.client.disable_rule(Name=self.name)
+        except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
+            self.module.fail_json_aws(e, msg="Could not disable rule %s" % self.name)
         self.changed = True
         return response
 
@@ -194,7 +209,9 @@ def list_targets(self):
             error_code = e.response.get('Error', {}).get('Code')
             if error_code == 'ResourceNotFoundException':
                 return []
-            raise
+            self.module.fail_json_aws(e, msg="Could not find target for rule %s" % self.name)
+        except botocore.exceptions.BotoCoreError as e:
+            self.module.fail_json_aws(e, msg="Could not find target for rule %s" % self.name)
         return self._snakify(targets)['targets']
 
     def put_targets(self, targets):
@@ -205,7 +222,10 @@ def put_targets(self, targets):
             'Rule': self.name,
             'Targets': self._targets_request(targets),
         }
-        response = self.client.put_targets(**request)
+        try:
+            response = self.client.put_targets(**request)
+        except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
+            self.module.fail_json_aws(e, msg="Could not create/update rule targets for rule %s" % self.name)
         self.changed = True
         return response
 
@@ -217,7 +237,10 @@ def remove_targets(self, target_ids):
             'Rule': self.name,
             'Ids': target_ids
         }
-        response = self.client.remove_targets(**request)
+        try:
+            response = self.client.remove_targets(**request)
+        except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
+            self.module.fail_json_aws(e, msg="Could not remove rule targets from rule %s" % self.name)
         self.changed = True
         return response
 
@@ -379,23 +402,25 @@ def get_cloudwatchevents_client(module):
                           resource='events',
                           region=region, endpoint=ec2_url,
                           **aws_conn_kwargs)
-    except boto3.exception.NoAuthHandlerFound as e:
+    except botocore.exceptions.ProfileNotFound as e:
         module.fail_json(msg=str(e))
 
 
 def main():
     argument_spec = ec2_argument_spec()
-    argument_spec.update(dict(
-        name                 = dict(required=True),
-        schedule_expression  = dict(),
-        event_pattern        = dict(),
-        state                = dict(choices=['present', 'disabled', 'absent'],
-                                    default='present'),
-        description          = dict(),
-        role_arn             = dict(),
-        targets              = dict(type='list', default=[]),
-    ))
-    module = AnsibleModule(argument_spec=argument_spec)
+    argument_spec.update(
+        dict(
+            name=dict(required=True),
+            schedule_expression=dict(),
+            event_pattern=dict(),
+            state=dict(choices=['present', 'disabled', 'absent'],
+                       default='present'),
+            description=dict(),
+            role_arn=dict(),
+            targets=dict(type='list', default=[]),
+        )
+    )
+    module = AnsibleAWSModule(argument_spec=argument_spec)
 
     if not HAS_BOTO3:
         module.fail_json(msg='boto3 required for this module')
diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt
index 4dc282a8af51a0..9313310fa41d3e 100644
--- a/test/sanity/pep8/legacy-files.txt
+++ b/test/sanity/pep8/legacy-files.txt
@@ -14,7 +14,6 @@ lib/ansible/modules/cloud/amazon/aws_kms.py
 lib/ansible/modules/cloud/amazon/cloudformation.py
 lib/ansible/modules/cloud/amazon/cloudformation_facts.py
 lib/ansible/modules/cloud/amazon/cloudfront_facts.py
-lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py
 lib/ansible/modules/cloud/amazon/dynamodb_table.py
 lib/ansible/modules/cloud/amazon/ec2_ami_copy.py
 lib/ansible/modules/cloud/amazon/ec2_ami_find.py


Code review comments:
diff_hunk: @@ -116,13 +116,12 @@
 '''  # NOQA
 
 try:
-    import boto3.exception
-    import botocore.exceptions
+    import botocore
 except ImportError:
     # module_utils.ec2.HAS_BOTO3 will do the right thing
     pass
 
-from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.aws.core import AnsibleAWSModule
 from ansible.module_utils.ec2 import (HAS_BOTO3, boto3_conn, camel_dict_to_snake_dict,
path: lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py
text: @user1:
Don't need to import HAS_BOTO3 now.

diff_hunk: @@ -379,23 +402,25 @@ def get_cloudwatchevents_client(module):
                           resource='events',
                           region=region, endpoint=ec2_url,
                           **aws_conn_kwargs)
-    except boto3.exception.NoAuthHandlerFound as e:
+    except botocore.exceptions.ProfileNotFound as e:
         module.fail_json(msg=str(e))
 
 
 def main():
     argument_spec = ec2_argument_spec()
-    argument_spec.update(dict(
-        name                 = dict(required=True),
-        schedule_expression  = dict(),
-        event_pattern        = dict(),
-        state                = dict(choices=['present', 'disabled', 'absent'],
-                                    default='present'),
-        description          = dict(),
-        role_arn             = dict(),
-        targets              = dict(type='list', default=[]),
-    ))
-    module = AnsibleModule(argument_spec=argument_spec)
+    argument_spec.update(
+        dict(
+            name=dict(required=True),
+            schedule_expression=dict(),
+            event_pattern=dict(),
+            state=dict(choices=['present', 'disabled', 'absent'],
+                       default='present'),
+            description=dict(),
+            role_arn=dict(),
+            targets=dict(type='list', default=[]),
+        )
+    )
+    module = AnsibleAWSModule(argument_spec=argument_spec)
 
     if not HAS_BOTO3:
path: lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py
text: @user1:
You can remove this since it's handled in module_utils/aws/core.py: https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/aws/core.py#L98-L100