General updates to the library

.pylintrc

@@ -179,7 +179,7 @@ ignore-imports=no
 [FORMAT]
 
 # Maximum number of characters on a single line.
-max-line-length=80
+max-line-length=120
 
 # Maximum number of lines in a module
 max-module-lines=1000

.travis.yml

@@ -2,7 +2,7 @@ language: python
 python:
   - "2.7"
 install:
-  - pip install -r test_requirements.txt
-script:  bash test.sh -q
+  - pip install -r requirements.txt -r test_requirements.txt
+script: py.test
 after_success:
   coveralls

gitreload/config.py

@@ -1,38 +1,45 @@
 """
 Setup configuration from a json file with defaults
 """
-import json
 import logging
 from logging.handlers import SysLogHandler
 import os
 import platform
 
 log = logging.getLogger('gitreload')  # pylint: disable=C0103
 
-CONFIG_PATHS = [
-    os.environ.get('GITRELOAD_CONFIG', ''),
-    os.path.join(os.getcwd(), 'gr.env.json'),
-    os.path.join(os.path.expanduser('~'), '.gr.env.json'),
-    '/etc/gr.env.json',
-]
+MINUTE = 60  # seconds
 
 
-def configure_logging(level_override=None):
+class Config(object):
+    """
+    Configuration for the app
+    """
+    REPODIR = os.environ.get('REPODIR', '/mnt/data/repos')
+    VIRTUAL_ENV = os.environ.get('VIRTUAL_ENV', '/edx/app/edxapp/venvs/edxapp')
+    DJANGO_SETTINGS = os.environ.get('DJANGO_SETTINGS', 'aws')
+    EDX_PLATFORM = os.environ.get('EDX_PLATFORM', '/edx/app/edxapp/edx-platform')
+    LINKED_REPOS = os.environ.get('LINKED_REPOS', {})
+    ALSO_CLONE_REPOS = os.environ.get('ALSO_CLONE_REPOS', {})
+    NUM_THREADS = int(os.environ.get('NUM_THREADS', 1))
+    LOG_LEVEL = os.environ.get('LOG_LEVEL', None)
+    HOSTNAME = platform.node().split('.')[0]
+    LOG_FORMATTER = ('%(asctime)s %(levelname)s %(process)d [%(name)s] '
+                     '%(filename)s:%(lineno)d - '
+                     '{hostname}- %(message)s').format(hostname=HOSTNAME)
+    LOG_FILE_PATH = os.environ.get('LOG_FILE_PATH', '')
+    SUBPROCESS_TIMEOUT = int(os.environ.get('SUBPROCESS_TIMEOUT_MINUTES', 60)) * MINUTE
+
+
+def configure_logging(level_override=None, config=Config):
     """
     Set the log level for the application
     """
 
-    # Set up format for default logging
-    hostname = platform.node().split('.')[0]
-    formatter = ('%(asctime)s %(levelname)s %(process)d [%(name)s] '
-                 '%(filename)s:%(lineno)d - '
-                 '{hostname}- %(message)s').format(hostname=hostname)
-
     set_level = level_override
 
-    # Grab config from settings if set, else allow system/language
-    # defaults.
-    config_log_level = settings.get('LOG_LEVEL', None)
+    # Grab config from settings if set, else allow system/language defaults.
+    config_log_level = config.LOG_LEVEL
     config_log_int = None
 
     if config_log_level and not set_level:
@@ -47,68 +54,24 @@ def configure_logging(level_override=None):
 
     # Setup logging with format and level (do setup incase we are
     # main, or change root logger if we aren't.
-    logging.basicConfig(level=level_override, format=formatter)
+    logging.basicConfig(level=level_override, format=config.LOG_FORMATTER)
     root_logger = logging.getLogger()
     root_logger.setLevel(set_level)
 
-    address = None
-    if os.path.exists('/dev/log'):
+    if config.LOG_FILE_PATH:
+        address = config.LOG_FILE_PATH
+    elif os.path.exists('/dev/log'):
         address = '/dev/log'
     elif os.path.exists('/var/run/syslog'):
         address = '/var/run/syslog'
     else:
-        address = ('127.0.0.1', 514)  # pylint: disable=redefined-variable-type
+        address = ('127.0.0.1', 514)  # pylint: disable=bad-option-value
     # Add syslog handler before adding formatters
     root_logger.addHandler(
         SysLogHandler(address=address, facility=SysLogHandler.LOG_LOCAL0)
     )
 
     for handler in root_logger.handlers:
-        handler.setFormatter(logging.Formatter(formatter))
+        handler.setFormatter(logging.Formatter(config.LOG_FORMATTER))
 
     return config_log_int
-
-
-def get_config():
-    """
-    Find and load the configuration file values
-    """
-    conf = {}
-    config_file = None
-    config = {}
-
-    for conf_path in CONFIG_PATHS:
-        if os.path.isfile(conf_path):
-            config_file = conf_path
-            break
-    if config_file:
-        with open(config_file) as env_file:
-            conf = json.load(env_file)
-
-    config['REPODIR'] = conf.get(
-        'REPODIR',
-        '/mnt/data/repos'
-    )
-    config['VIRTUAL_ENV'] = conf.get(
-        'VIRTUAL_ENV',
-        '/edx/app/edxapp/venvs/edxapp'
-    )
-    config['DJANGO_SETTINGS'] = conf.get(
-        'DJANGO_SETTINGS',
-        'aws'
-    )
-    config['EDX_PLATFORM'] = conf.get(
-        'EDX_PLATFORM',
-        '/edx/app/edxapp/edx-platform'
-    )
-    config['LOG_LEVEL'] = conf.get(
-        'LOG_LEVEL',
-        None
-    )
-    config['LINKED_REPOS'] = conf.get('LINKED_REPOS', {})
-    config['ALSO_CLONE_REPOS'] = conf.get('ALSO_CLONE_REPOS', {})
-    config['NUM_THREADS'] = int(conf.get('NUM_THREADS', 1))
-
-    return config
-
-settings = get_config()  # pylint: disable=C0103

gitreload/processing.py

@@ -6,11 +6,11 @@
 import logging
 import multiprocessing
 import os
-import subprocess
+import subprocess32
 
 from git import Repo
 
-from gitreload.config import settings
+from gitreload import config
 
 log = logging.getLogger('gitreload')  # pylint: disable=C0103
 
@@ -22,26 +22,29 @@ def import_repo(action_call):
     """
     os.environ['SERVICE_VARIANT'] = 'lms'
     cmd = [
-        '{0}/bin/python'.format(settings['VIRTUAL_ENV']),
+        '{0}/bin/python'.format(config.Config.VIRTUAL_ENV),
         'manage.py',
         'lms',
-        '--settings={0}'.format(settings['DJANGO_SETTINGS']),
+        '--settings={0}'.format(config.Config.DJANGO_SETTINGS),
         'git_add_course',
         action_call.repo_url,
         '--directory_path',
-        os.path.join(settings['REPODIR'], action_call.repo_name),
+        os.path.join(config.Config.REPODIR, action_call.repo_name),
     ]
 
     log.info('Beginning import of course repo %s with command %s',
              action_call.repo_name, ' '.join(cmd))
     try:
-        import_process = subprocess.check_output(
+        import_process = subprocess32.check_output(
             cmd,
-            cwd=settings['EDX_PLATFORM'],
-            stderr=subprocess.STDOUT
+            cwd=config.Config.EDX_PLATFORM,
+            stderr=subprocess32.STDOUT,
+            timeout=config.Config.SUBPROCESS_TIMEOUT,
         )
-    except subprocess.CalledProcessError as ex:
-        log.exception('Import command failed with: %s', ex.output)
+    except subprocess32.CalledProcessError as exc:
+        log.exception('Import command failed with: %s', exc.output)
+    except subprocess32.TimeoutExpired as exc:
+        log.exception('Import command timed out after %s seconds with: %s', exc.timeout, exc.output)
     except OSError as ex:
         log.exception('System or configuration error occurred: %s', str(ex))
     else:
@@ -54,7 +57,7 @@ def git_get_latest(action_call):
     and `git reset --hard origin/<repo_branch>`
     on the passed in repo.
     """
-    repo = Repo(os.path.join(settings['REPODIR'], action_call.repo_name))
+    repo = Repo(os.path.join(config.Config.REPODIR, action_call.repo_name))
     # Grab HEAD sha to see if we actually are updating
     orig_head = repo.head.commit.tree.hexsha
     repo.git.fetch('--all')
@@ -169,7 +172,7 @@ def run(self):  # pragma: no cover due to multiprocessing
                           action_call.action_type)
                 self.ACTION_COMMANDS[action_call.action_type](action_call)
             except Exception:  # pylint: disable=W0703
-                log.exception('Failed to run command')
+                log.exception('Failed to run command GitAction')
             finally:
                 self.queued_jobs.pop()
                 self.queue.task_done()

gitreload/tests/test_config.py

@@ -1,128 +1,77 @@
 """
 Unit tests to validate configuration loading
 """
+import logging
 import os
 import unittest
 import mock
 
-from .base import TEST_ROOT
-
 TEST_NUM_THREADS = 10
 TEST_LOG_LEVEL = 'DEBUG'
 
 
-@mock.patch('gitreload.config.CONFIG_PATHS')
-class TestConfiguration(unittest.TestCase):
+class TestLogConfiguration(unittest.TestCase):
     """
-    Test out configuration defaults, loading json config, etc.
+    Make sure we are setting up logging like we expect.
     """
     # pylint: disable=R0904
 
-    DEFAULT_CONFIG_EXPECT = {
-        'REPODIR': '/mnt/data/repos',
-        'VIRTUAL_ENV': '/edx/app/edxapp/venvs/edxapp',
-        'DJANGO_SETTINGS': 'aws',
-        'EDX_PLATFORM': '/edx/app/edxapp/edx-platform',
-        'LOG_LEVEL': None,
-        'LINKED_REPOS': {},
-        'ALSO_CLONE_REPOS': {},
-        'NUM_THREADS': 1,
-    }
-
-    def test_defaults(self, config_path_mock):
-        """
-        Simply validate that config settings are what we expect above
-        """
-        config_path_mock.__iter__.return_value = []
-        from gitreload.config import get_config
-        self.assertEqual(get_config(), self.DEFAULT_CONFIG_EXPECT)
-
-    def test_overrides(self, config_path_mock):
-        """
-        Load our test config and make sure things are working right
-        """
-        config_path_mock.__iter__.return_value = [
-            os.path.join(TEST_ROOT, 'gr.env.json'),
-        ]
-        from gitreload.config import get_config
-        local_settings = get_config()
-        self.assertEqual(local_settings['NUM_THREADS'], TEST_NUM_THREADS)
-        self.assertEqual(local_settings['LOG_LEVEL'], TEST_LOG_LEVEL)
-
-    def test_bad_json(self, config_path_mock):
+    def setUp(self):
         """
-        Load up a bad json file to make sure that we raise and exit
+        Set up method
         """
-        config_path_mock.__iter__.return_value = [
-            os.path.join(TEST_ROOT, 'gr.env.bad.json'),
-        ]
-        from gitreload.config import get_config
-        with self.assertRaisesRegexp(ValueError,
-                                     "No JSON object could be decoded"):
-            get_config()
-
-
-class TestLogConfiguration(unittest.TestCase):
-    """
-    Make sure we are setting up logging like we expect.
-    """
-    # pylint: disable=R0904
+        # reset the log level before each test
+        logger = logging.getLogger()
+        logger.setLevel(logging.WARNING)
 
     def test_log_override(self):
         """
         Make sure we can setup logging with our own level
         """
-        import logging
         root_logger = logging.getLogger()
         log_level = root_logger.level
-        self.assertEqual(logging.NOTSET, log_level)
+        self.assertEqual(logging.WARNING, log_level)
 
         from gitreload.config import configure_logging
         log_level = configure_logging(logging.INFO)
         root_logger = logging.getLogger()
         self.assertEqual(root_logger.level, logging.INFO)
 
-    @mock.patch.dict('gitreload.config.settings',
-                     {'LOG_LEVEL': TEST_LOG_LEVEL})
+    @mock.patch('gitreload.config.Config.LOG_LEVEL', TEST_LOG_LEVEL)
     def test_config_log_level(self,):
         """
         Patch config and make sure we are setting to it
         """
-        import logging
         root_logger = logging.getLogger()
         log_level = root_logger.level
-        self.assertEqual(logging.NOTSET, log_level)
+        self.assertEqual(logging.WARNING, log_level)
 
         from gitreload.config import configure_logging
         log_level = configure_logging()
         root_logger = logging.getLogger()
         self.assertEqual(root_logger.level, getattr(logging, TEST_LOG_LEVEL))
 
-    @mock.patch.dict('gitreload.config.settings',
-                     {'LOG_LEVEL': 'Not a real thing'})
+    @mock.patch('gitreload.config.Config.LOG_LEVEL', 'Not a real thing')
     def test_bad_log_level(self,):
         """
         Set a non-existent log level and make sure we raise properly
         """
-        import logging
         root_logger = logging.getLogger()
         log_level = root_logger.level
-        self.assertEqual(logging.NOTSET, log_level)
+        self.assertEqual(logging.WARNING, log_level)
 
         from gitreload.config import configure_logging
         with self.assertRaisesRegexp(ValueError, 'Invalid log level.+'):
             log_level = configure_logging()
 
-    @mock.patch.dict('gitreload.config.settings',
-                     {'LOG_LEVEL': None})
+    @mock.patch('gitreload.config.Config.LOG_LEVEL', None)
     def test_no_log_level(self):
         """
         Make sure we leave things alone if no log level is set.
         """
-        import logging
         root_logger = logging.getLogger()
         log_level = root_logger.level
-        self.assertEqual(logging.NOTSET, log_level)
+        self.assertEqual(logging.WARNING, log_level)
 
         from gitreload.config import configure_logging
         log_level = configure_logging()
@@ -132,7 +81,6 @@ def test_syslog_devices(self):
         """
         Test syslog address handling and handler
         """
-        import logging
 
         for log_device in ['/dev/log', '/var/run/syslog', '']:
             root_logger = logging.getLogger()