Merge pull request #32 from mitodl/feature/pylint

Pylint cleanup

orcoursetrion/actions/github.py

@@ -1,4 +1,6 @@
 # -*- coding: utf-8 -*-
+# Because pylint doesn't do dynamic attributes for orcoursetrion.config
+# pylint: disable=no-member
 """
 Github based actions for orchestrion to take. i.e. "Create a studio
 course export repo", "Add course team to github", etc

orcoursetrion/config.py

@@ -4,18 +4,6 @@
 """
 import os
 
-prefer_django = True
-try:
-    from django.conf import settings
-except ImportError:
-    prefer_django = False
-
-if prefer_django:
-    primary_config = settings
-    fallback_config = os.environ
-else:
-    primary_config = os.environ
-    fallback_config = os.environ
 
 CONFIG_KEYS = {
     # GitHub API Key
@@ -51,6 +39,30 @@
     # Web hook URL (including basic auth) for course production LMS
     'ORC_PRODUCTION_GITRELOAD': None,
 }
-for key, default_value in CONFIG_KEYS.items():
-    value = primary_config.get(key, fallback_config.get(key, default_value))
-    vars()[key] = value
+
+
+def configure():
+    """
+    Configure the application using a three way try for settings.
+    """
+    prefer_django = True
+    try:
+        from django.conf import settings
+    except ImportError:
+        prefer_django = False
+
+    if prefer_django:
+        primary_config = settings
+        fallback_config = os.environ
+    else:
+        primary_config = os.environ
+        fallback_config = os.environ
+
+    for key, default_value in CONFIG_KEYS.items():
+        value = primary_config.get(
+            key,
+            fallback_config.get(key, default_value)
+        )
+        globals()[key] = value
+
+configure()

orcoursetrion/lib/github.py

@@ -10,6 +10,9 @@
 import sh
 
 
+CLONE_DIR = 'cloned_repo'
+
+
 class GitHubException(Exception):
     """Base exception class others inherit."""
     pass
@@ -237,6 +240,9 @@ def put_team(self, org, team_name, read_only, members):
                 (https://developer.github.com/v3/orgs/teams/#response-1)
 
         """
+        # Disabling too-many-locals because I need them as a human to
+        # keep track of the sets going on here.
+        # pylint: disable=too-many-locals
         try:
             team_dict = self._find_team(org, team_name)
         except GitHubNoTeamFound:
@@ -394,7 +400,8 @@ def delete_web_hooks(self, org, repo):
             num_hooks_removed += 1
         return num_hooks_removed
 
-    def shallow_copy_repo(self, src_repo, dst_repo, committer, branch=None):
+    @staticmethod
+    def shallow_copy_repo(src_repo, dst_repo, committer, branch=None):
         """Copies one branch repo's contents to a new repo in the same
         organization without history.
 
@@ -424,8 +431,10 @@ def shallow_copy_repo(self, src_repo, dst_repo, committer, branch=None):
             None
 
         """
-        CLONE_DIR = 'cloned_repo'
-        # Grab current working directoy so we return after we are done
+        # Disable member use because pylint doesn't get dynamic members
+        # pylint: disable=no-member
+
+        # Grab current working directory so we return after we are done
         cwd = unicode(sh.pwd().rstrip('\n'))
         tmp_dir = tempfile.mkdtemp(prefix='orc_git')
         try:

orcoursetrion/tests/__init__.py

@@ -0,0 +1,3 @@
+"""
+All the test classes for orcoursetrion
+"""

orcoursetrion/tests/base.py

@@ -46,6 +46,9 @@ def callback_repo_check(self, request, uri, headers, status_code=404):
 
     def callback_repo_create(self, request, uri, headers, status_code=201):
         """Mock repo creation API call."""
+        # Disabling unused-argument because this is a callback with
+        # required method signature.
+        # pylint: disable=unused-argument
         self.assertEqual(
             request.headers['Authorization'],
             'token {0}'.format(self.OAUTH2_TOKEN)
@@ -63,6 +66,8 @@ def callback_team_list(
             self, request, uri, headers, status_code=200, more=False
     ):
         """Mock team listing API call."""
+        # All arguments needed for tests
+        # pylint: disable=too-many-arguments
         self.assertEqual(
             request.headers['Authorization'],
             'token {0}'.format(self.OAUTH2_TOKEN)
@@ -96,11 +101,16 @@ def callback_team_list(
 
     def callback_team_members(
             self, request, uri, headers,
-            status_code=200, members=TEST_TEAM_MEMBERS
+            status_code=200, members=None
     ):
         """
         Return team membership list
         """
+        # Disabling unused-argument because this is a callback with
+        # required method signature.
+        # pylint: disable=unused-argument,too-many-arguments
+        if members is None:
+            members = self.TEST_TEAM_MEMBERS
         self.assertEqual(
             request.headers['Authorization'],
             'token {0}'.format(self.OAUTH2_TOKEN)
@@ -115,6 +125,9 @@ def callback_team_create(
         """
         Create a new team as requested
         """
+        # Disabling unused-argument because this is a callback with
+        # required method signature.
+        # pylint: disable=unused-argument,too-many-arguments
         self.assertEqual(
             request.headers['Authorization'],
             'token {0}'.format(self.OAUTH2_TOKEN)
@@ -128,15 +141,18 @@ def callback_team_create(
             self.assertEqual(json_body['permission'], 'push')
         return (status_code, headers, json.dumps({'id': 2}))
 
+    @staticmethod
     def callback_team_membership(
-            self, request, uri, headers, success=True, action_list=None
+        request, uri, headers, success=True, action_list=None
     ):
         """Manage both add and delete of team membership.
 
         ``action_list`` is a list of tuples with (``username``,
         ``added (bool)``) to track state of membership since this will
         get called multiple times in one library call.
         """
+        # pylint: disable=too-many-arguments
+
         username = uri.rsplit('/', 1)[1]
         if not success:
             status_code = 500
@@ -159,7 +175,6 @@ def callback_team_repo(self, request, uri, headers, status_code=204):
         self.assertIsNotNone(re.match(
             '{url}teams/[13]/repos/{org}/({repo}|{rerun_repo})'.format(
                 url=re.escape(self.URL),
-                id=self.TEST_TEAM_ID,
                 org=self.ORG,
                 repo=re.escape(self.TEST_REPO),
                 rerun_repo=re.escape(self.TEST_RERUN_REPO)
@@ -290,7 +305,7 @@ def register_team_members(self, body):
         httpretty.register_uri(
             httpretty.GET,
             re.compile(
-                '^{url}teams/\d+/members$'.format(
+                r'^{url}teams/\d+/members$'.format(
                     url=re.escape(self.URL)
                 )
             ),
@@ -301,7 +316,7 @@ def register_team_membership(self, body):
         """
         Register adding and removing team members.
         """
-        url_regex = re.compile('^{url}teams/\d+/memberships/\w+$'.format(
+        url_regex = re.compile(r'^{url}teams/\d+/memberships/\w+$'.format(
             url=re.escape(self.URL),
         ))
         httpretty.register_uri(
@@ -318,7 +333,7 @@ def register_team_repo_add(self, body):
         httpretty.register_uri(
             httpretty.PUT,
             re.compile(
-                '^{url}teams/\d+/repos/{org}/({repo}|{rerun_repo})$'.format(
+                r'^{url}teams/\d+/repos/{org}/({repo}|{rerun_repo})$'.format(
                     url=self.URL,
                     org=self.ORG,
                     repo=re.escape(self.TEST_REPO),

orcoursetrion/tests/test_core.py

@@ -10,9 +10,10 @@
 class TestCore(unittest.TestCase):
     """Test common and core components to the package.
     """
+    # pylint: disable=too-few-public-methods
 
-    def test_version(self):
+    @staticmethod
+    def test_version():
         """Validate version matches proper format"""
-        # Will raise ValueError if not a semantic version
         import orcoursetrion
         semantic_version.Version(orcoursetrion.VERSION)

orcoursetrion/tests/test_github.py

@@ -2,6 +2,9 @@
 """
 Test github actions and backing library
 """
+# Because pylint can't figure out dynamic attributes for config or sh
+# pylint: disable=no-member
+
 from functools import partial
 import json
 import os
@@ -76,6 +79,7 @@ def test_lib_get_all_bad_status(self):
         )
         git_hub = GitHub(self.URL, self.OAUTH2_TOKEN)
         with self.assertRaisesRegexp(GitHubUnknownError, re.escape(error)):
+            # pylint: disable=protected-access
             git_hub._get_all(test_url)
 
     @httpretty.activate
@@ -198,8 +202,8 @@ def test_lib_add_team_repo_no_teams(self):
         git_hub = GitHub(self.URL, self.OAUTH2_TOKEN)
         # See how we handle no teams
         with self.assertRaisesRegexp(
-                GitHubUnknownError,
-                re.escape("No teams found in org. This shouldn't happen")
+            GitHubUnknownError,
+            re.escape("No teams found in org. This shouldn't happen")
         ):
             git_hub.add_team_repo(self.ORG, self.TEST_REPO, self.TEST_TEAM)
 
@@ -241,7 +245,7 @@ def test_lib_add_team_repo_fail(self):
 
         git_hub = GitHub(self.URL, self.OAUTH2_TOKEN)
         with self.assertRaisesRegexp(GitHubUnknownError, json.dumps({
-                "message": "Validation Failed",
+            "message": "Validation Failed",
         })):
             git_hub.add_team_repo(self.ORG, self.TEST_REPO, self.TEST_TEAM)
 
@@ -323,7 +327,7 @@ def test_lib_put_team_creation_fail(self):
         )
         git_hub = GitHub(self.URL, self.OAUTH2_TOKEN)
         with self.assertRaisesRegexp(
-                GitHubUnknownError, json.dumps({'id': 2})
+            GitHubUnknownError, json.dumps({'id': 2})
         ):
             git_hub.put_team(
                 self.ORG, 'New Team', True, self.TEST_TEAM_MEMBERS
@@ -339,22 +343,24 @@ def test_lib_put_team_membership_fail(self):
         )
         git_hub = GitHub(self.URL, self.OAUTH2_TOKEN)
         with self.assertRaisesRegexp(
-                GitHubUnknownError, '^Failed to add or remove.+$'
+            GitHubUnknownError, '^Failed to add or remove.+$'
         ):
             git_hub.put_team(self.ORG, self.TEST_TEAM, True, [])
 
     def test_lib_copy_repo(self):
         """Verify that we can do a single commit, single branch copy of a
         repo."""
+        # Even pylint thinks this test is too long, but it is needed
+        # pylint: disable=too-many-locals
 
         from orcoursetrion import config
 
         commit_1 = 'hello'
         commit_2 = 'world'
         branch = 'foo'
 
-        SRC_REPO = 'Thing1'
-        DST_REPO = 'Thing2'
+        src_repo = 'Thing1'
+        dst_repo = 'Thing2'
 
         prefix = 'orc_git_test'
 
@@ -379,9 +385,9 @@ def test_lib_copy_repo(self):
         # Create a base repo with some commits, then
         # Run the copy and verify the results
         sh.cd(tmp_dir_src)
-        sh.mkdir(SRC_REPO)
-        sh.cd(SRC_REPO)
-        git = sh.git.bake(_cwd=os.path.join(tmp_dir_src, SRC_REPO))
+        sh.mkdir(src_repo)
+        sh.cd(src_repo)
+        git = sh.git.bake(_cwd=os.path.join(tmp_dir_src, src_repo))
         git.init()
         git.config('user.email', config.ORC_GH_EMAIL)
         git.config('user.name', config.ORC_GH_NAME)
@@ -402,15 +408,15 @@ def test_lib_copy_repo(self):
 
         # Now create the initial bare repo at the destination
         sh.cd(tmp_dir_dst)
-        sh.mkdir(DST_REPO)
-        sh.cd(DST_REPO)
+        sh.mkdir(dst_repo)
+        sh.cd(dst_repo)
         sh.git.init(bare=True)
 
         # Alright, run the copy and verify the expected commit
         # message, and that the test file has the last commit string
         git_hub = GitHub(self.URL, self.OAUTH2_TOKEN)
-        src_repo_url = 'file://{0}'.format(os.path.join(tmp_dir_src, SRC_REPO))
-        dst_repo_url = 'file://{0}'.format(os.path.join(tmp_dir_dst, DST_REPO))
+        src_repo_url = 'file://{0}'.format(os.path.join(tmp_dir_src, src_repo))
+        dst_repo_url = 'file://{0}'.format(os.path.join(tmp_dir_dst, dst_repo))
 
         git_hub.shallow_copy_repo(
             src_repo_url,
@@ -421,9 +427,9 @@ def test_lib_copy_repo(self):
         # Verify things are looking right.
         sh.cd(tmp_dir_dst)
         # Clone bare destination repo and verify contents
-        DST_REPO_CLONE = 'cloned'
-        sh.git.clone(dst_repo_url, DST_REPO_CLONE)
-        sh.cd(DST_REPO_CLONE)
+        dst_repo_clone = 'cloned'
+        sh.git.clone(dst_repo_url, dst_repo_clone)
+        sh.cd(dst_repo_clone)
 
         # Assert file is as expected
         with open('test', 'r') as test_file:
@@ -450,7 +456,7 @@ def test_lib_copy_repo(self):
 
         # Delete old clone and run it with a different branch
         sh.cd(tmp_dir_dst)
-        shutil.rmtree(DST_REPO_CLONE)
+        shutil.rmtree(dst_repo_clone)
         git_hub.shallow_copy_repo(
             src_repo_url,
             dst_repo_url,
@@ -461,9 +467,9 @@ def test_lib_copy_repo(self):
         # Verify things are looking right in the branch clone
         sh.cd(tmp_dir_dst)
         # Clone bare destination repo and verify contents
-        DST_REPO_CLONE = 'cloned'
-        sh.git.clone(dst_repo_url, DST_REPO_CLONE)
-        sh.cd(DST_REPO_CLONE)
+        dst_repo_clone = 'cloned'
+        sh.git.clone(dst_repo_url, dst_repo_clone)
+        sh.cd(dst_repo_clone)
 
         # Assert file is as expected
         with open('test', 'r') as test_file: