Refactor github_hook module (#24391)

* PEP8 fixes
* Refactoring for fetch_url call
* Removed un-used code

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

lib/ansible/modules/source_control/github_hooks.py

@@ -26,7 +26,7 @@ ANSIBLE_METADATA = {'metadata_version': '1.0',
 DOCUMENTATION = '''
 ---
 module: github_hooks
-short_description: Manages github service hooks.
+short_description: Manages GitHub service hooks.
 description:
      - Adds service hooks and removes service hooks that have an error status.
 version_added: "1.4"
@@ -37,7 +37,7 @@ options:
     required: true
   oauthkey:
     description:
-      - The oauth key provided by github. It can be found/generated on github under "Edit Your Profile" >> "Applications" >> "Personal Access Tokens"
+      - The oauth key provided by GitHub. It can be found/generated on GitHub under "Edit Your Profile" >> "Developer settings" >> "Personal Access Tokens"
     required: true
   repo:
     description:
@@ -47,7 +47,7 @@ options:
     required: true
   hookurl:
     description:
-      - When creating a new hook, this is the url that you want github to post to. It is only required when creating a new hook.
+      - When creating a new hook, this is the url that you want GitHub to post to. It is only required when creating a new hook.
     required: false
   action:
     description:
@@ -100,44 +100,50 @@ except ImportError:
         pass
 
 import base64
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.urls import fetch_url
 
 
-def _list(module, hookurl, oauthkey, repo, user):
+def request(module, url, user, oauthkey, data='', method='GET'):
-    url = "%s/hooks" % repo
     auth = base64.encodestring('%s:%s' % (user, oauthkey)).replace('\n', '')
     headers = {
         'Authorization': 'Basic %s' % auth,
     }
-    response, info = fetch_url(module, url, headers=headers)
+    response, info = fetch_url(module, url, headers=headers, data=data, method=method)
+    return response, info
+
+
+def _list(module, oauthkey, repo, user):
+    url = "%s/hooks" % repo
+    response, info = request(module, url, user, oauthkey)
     if info['status'] != 200:
         return False, ''
     else:
         return False, response.read()
 
-def _clean504(module, hookurl, oauthkey, repo, user):
+
-    current_hooks = _list(hookurl, oauthkey, repo, user)[1]
+def _clean504(module, oauthkey, repo, user):
+    current_hooks = _list(module, oauthkey, repo, user)[1]
     decoded = json.loads(current_hooks)
 
     for hook in decoded:
         if hook['last_response']['code'] == 504:
-            # print "Last response was an ERROR for hook:"
+            _delete(module, oauthkey, repo, user, hook['id'])
-            # print hook['id']
-            _delete(module, hookurl, oauthkey, repo, user, hook['id'])
 
     return 0, current_hooks
 
-def _cleanall(module, hookurl, oauthkey, repo, user):
+
-    current_hooks = _list(hookurl, oauthkey, repo, user)[1]
+def _cleanall(module, oauthkey, repo, user):
+    current_hooks = _list(module, oauthkey, repo, user)[1]
     decoded = json.loads(current_hooks)
 
     for hook in decoded:
         if hook['last_response']['code'] != 200:
-            # print "Last response was an ERROR for hook:"
+            _delete(module, oauthkey, repo, user, hook['id'])
-            # print hook['id']
-            _delete(module, hookurl, oauthkey, repo, user, hook['id'])
 
     return 0, current_hooks
 
+
 def _create(module, hookurl, oauthkey, repo, user, content_type):
     url = "%s/hooks" % repo
     values = {
@@ -149,25 +155,19 @@ def _create(module, hookurl, oauthkey, repo, user, content_type):
         }
     }
     data = json.dumps(values)
-    auth = base64.encodestring('%s:%s' % (user, oauthkey)).replace('\n', '')
+    response, info = request(module, url, user, oauthkey, data=data, method='POST')
-    headers = {
-        'Authorization': 'Basic %s' % auth,
-    }
-    response, info = fetch_url(module, url, data=data, headers=headers)
     if info['status'] != 200:
         return 0, '[]'
     else:
         return 0, response.read()
 
-def _delete(module, hookurl, oauthkey, repo, user, hookid):
+
+def _delete(module, oauthkey, repo, user, hookid):
     url = "%s/hooks/%s" % (repo, hookid)
-    auth = base64.encodestring('%s:%s' % (user, oauthkey)).replace('\n', '')
+    response, info = request(module, url, user, oauthkey, method='DELETE')
-    headers = {
-        'Authorization': 'Basic %s' % auth,
-    }
-    response, info = fetch_url(module, url, data=data, headers=headers, method='DELETE')
     return response.read()
 
+
 def main():
     module = AnsibleModule(
         argument_spec=dict(
@@ -189,13 +189,13 @@ def main():
     content_type = module.params['content_type']
 
     if action == "list":
-        (rc, out) = _list(module, hookurl, oauthkey, repo, user)
+        (rc, out) = _list(module, oauthkey, repo, user)
 
     if action == "clean504":
-        (rc, out) = _clean504(module, hookurl, oauthkey, repo, user)
+        (rc, out) = _clean504(module, oauthkey, repo, user)
 
     if action == "cleanall":
-        (rc, out) = _cleanall(module, hookurl, oauthkey, repo, user)
+        (rc, out) = _cleanall(module, oauthkey, repo, user)
 
     if action == "create":
         (rc, out) = _create(module, hookurl, oauthkey, repo, user, content_type)
@@ -206,9 +206,5 @@ def main():
     module.exit_json(msg="success", result=out)
 
 
-# import module snippets
-from ansible.module_utils.basic import *
-from ansible.module_utils.urls import *
-
 if __name__ == '__main__':
     main()

test/sanity/pep8/legacy-files.txt

@@ -499,7 +499,6 @@ lib/ansible/modules/remote_management/hpilo/hponcfg.py
 lib/ansible/modules/remote_management/stacki/stacki_host.py
 lib/ansible/modules/remote_management/wakeonlan.py
 lib/ansible/modules/source_control/bzr.py
-lib/ansible/modules/source_control/github_hooks.py
 lib/ansible/modules/source_control/gitlab_group.py
 lib/ansible/modules/source_control/gitlab_project.py
 lib/ansible/modules/source_control/gitlab_user.py