Feature/cg/sysadmin dashboard mongook

lms/djangoapps/dashboard/management/commands/git_add_course.py

@@ -6,6 +6,7 @@
 import re
 import datetime
 import StringIO
+import subprocess
 import logging
 
 from django.conf import settings
@@ -70,53 +71,53 @@ def add_repo(repo, rdir_in):
     if os.path.exists(rdirp):
         log.info(_('directory already exists, doing a git pull instead '
                    'of git clone'))
-        cmd = 'cd {0}/{1}; git pull'.format(GIT_REPO_DIR, rdir)
+        cmd = ['git', 'pull', ]
+        cwd = '{0}/{1}'.format(GIT_REPO_DIR, rdir)
     else:
-        cmd = 'cd {0}; git clone "{1}"'.format(GIT_REPO_DIR, repo)
+        cmd = ['git', 'clone', repo, ]
+        cwd = GIT_REPO_DIR
 
     log.debug(cmd)
-    ret_git = os.popen(cmd).read()
+    cwd = os.path.abspath(cwd)
+    ret_git = subprocess.check_output(cmd, cwd=cwd)
     log.debug(ret_git)
 
     if not os.path.exists('{0}/{1}'.format(GIT_REPO_DIR, rdir)):
         log.error(_('git clone failed!'))
         return -1
 
     # get commit id
-    commit_id = os.popen('cd {0}; git log -n 1 | head -1'.format(
-        rdirp)).read().strip().split(' ')[1]
+    cmd = ['git', 'log', '-1', '--format=%H', ]
+    commit_id = subprocess.check_output(cmd, cwd=rdirp)
 
     ret_git += _('\nCommit ID: {0}').format(commit_id)
 
     # get branch
-    branch = ''
-    for k in os.popen('cd {0}; git branch'.format(rdirp)).readlines():
-        if k[0] == '*':
-            branch = k[2:].strip()
-
+    cmd = ['git', 'rev-parse', '--abbrev-ref', 'HEAD', ]
+    branch = subprocess.check_output(cmd, cwd=rdirp)
     ret_git += '   \nBranch: {0}'.format(branch)
 
     # Get XML logging logger and capture debug to parse results
     output = StringIO.StringIO()
-    import_logger = logging.getLogger('xmodule.modulestore.xml_importer')
-    git_logger = logging.getLogger('git_add_script')
-    xml_logger = logging.getLogger('xmodule.modulestore.xml')
-    xml_seq_logger = logging.getLogger('xmodule.seq_module')
-
     import_log_handler = logging.StreamHandler(output)
     import_log_handler.setLevel(logging.DEBUG)
 
-    for logger in [import_logger, git_logger, xml_logger, xml_seq_logger, ]:
+    logger_names = ['xmodule.modulestore.xml_importer', 'git_add_course',
+                    'xmodule.modulestore.xml', 'xmodule.seq_module', ]
+    loggers = []
+
+    for logger_name in logger_names:
+        logger = logging.getLogger(logger_name)
         logger.old_level = logger.level
         logger.setLevel(logging.DEBUG)
         logger.addHandler(import_log_handler)
+        loggers.append(logger)
 
     try:
         management.call_command('import', GIT_REPO_DIR, rdir,
                                 nostatic=not GIT_IMPORT_STATIC)
-    except CommandError, ex:
-        log.critical(_('Unable to run import command.'))
-        log.critical(_('Error was {0}').format(str(ex)))
+    except CommandError:
+        log.exception(_('Unable to run import command.'))
         return -1
     except NotImplementedError, ex:
         log.critical(_('The underlying module store does not support import.'))
@@ -125,7 +126,7 @@ def add_repo(repo, rdir_in):
     ret_import = output.getvalue()
 
     # Remove handler hijacks
-    for logger in [import_logger, git_logger, xml_logger, xml_seq_logger, ]:
+    for logger in loggers:
         logger.setLevel(logger.old_level)
         logger.removeHandler(import_log_handler)
 
@@ -147,14 +148,21 @@ def add_repo(repo, rdir_in):
 
         if os.path.exists(cdir) and not os.path.islink(cdir):
             log.debug(_('   -> exists, but is not symlink'))
-            log.debug(os.popen('ls -l {0}'.format(cdir)).read())
-            log.debug(os.popen('rmdir {0}'.format(cdir)).read())
+            log.debug(subprocess.check_output(['ls', 'l', ],
+                                              cwd=os.path.abspath(cdir)))
+            try:
+                os.rmdir(os.path.abspath(cdir))
+            except OSError:
+                log.exception(_('Failed to remove course directory'))
 
         if not os.path.exists(cdir):
-            log.debug(_('   -> creating symlink'))
-            log.debug(os.popen('ln -s {0} {1}'.format(rdirp,
-                      cdir)).read())
-            log.debug(os.popen('ls -l {0}'.format(cdir)).read())
+            log.debug(_('   -> creating symlink between {0} and {1}').format(rdirp, cdir))
+            try:
+                os.symlink(os.path.abspath(rdirp), os.path.abspath(cdir))
+            except OSError:
+                log.exception(_('Unable to create course symlink'))
+            log.debug(subprocess.check_output(['ls', '-l', ],
+                                              cwd=os.path.abspath(cdir)))
 
     # store import-command-run output in mongo
     mongouri = 'mongodb://{0}:{1}@{2}/{3}'.format(
@@ -166,10 +174,9 @@ def add_repo(repo, rdir_in):
             mdb = mongoengine.connect(mongo_db['db'], host=mongouri)
         else:
             mdb = mongoengine.connect(mongo_db['db'], host=mongo_db['host'])
-    except mongoengine.connection.ConnectionError, ex:
-        log.critical(_('Unable to connect to mongodb to save log, please '
-                       'check MONGODB_LOG settings'))
-        log.critical(_('Error was: {0}').format(str(ex)))
+    except mongoengine.connection.ConnectionError:
+        log.exception(_('Unable to connect to mongodb to save log, please '
+                        'check MONGODB_LOG settings'))
         return -1
     cil = CourseImportLog(
         course_id=course_id,

lms/djangoapps/dashboard/sysadmin.py

@@ -74,7 +74,7 @@ def dispatch(self, *args, **kwargs):
         return super(SysadminDashboardView, self).dispatch(*args, **kwargs)
 
     def get_courses(self):
-        """ Get an iterable list of courses regardless of module store type."""
+        """ Get an iterable list of courses."""
 
         courses = self.def_ms.get_courses()
         courses = dict([c.id, c] for c in courses)  # no course directory
@@ -372,27 +372,29 @@ def import_mongo_course(self, gitloc):
 
         # Grab logging output for debugging imports
         output = StringIO.StringIO()
-
-        import_logger = logging.getLogger('xmodule.modulestore.xml_importer')
-        git_logger = logging.getLogger('dashboard.management.commands.git_add_course')
-        xml_logger = logging.getLogger('xmodule.modulestore.xml')
-        xml_seq_logger = logging.getLogger('xmodule.seq_module')
-
         import_log_handler = logging.StreamHandler(output)
         import_log_handler.setLevel(logging.DEBUG)
 
-        for logger in [import_logger, git_logger, xml_logger, xml_seq_logger, ]:
+        logger_names = ['xmodule.modulestore.xml_importer',
+                        'dashboard.management.commands.git_add_course',
+                        'xmodule.modulestore.xml', 'xmodule.seq_module', ]
+        loggers = []
+
+        for logger_name in logger_names:
+            logger = logging.getLogger(logger_name)
             logger.old_level = logger.level
             logger.setLevel(logging.DEBUG)
             logger.addHandler(import_log_handler)
+            loggers.append(logger)
 
         git_add_course.add_repo(gitloc, None)
         ret = output.getvalue()
 
         # Remove handler hijacks
-        for logger in [import_logger, git_logger, xml_logger, xml_seq_logger, ]:
+        for logger in loggers:
             logger.setLevel(logger.old_level)
             logger.removeHandler(import_log_handler)
+
         msg = u"<h4 style='color:blue'>{0} {1}</h4>".format(
             _('Added course from'), gitloc)
         msg += _("<pre>{0}</pre>").format(escape(ret))
@@ -413,10 +415,14 @@ def import_xml_course(self, gitloc, datatable):
         if os.path.exists(gdir):
             msg += _("The course {0} already exists in the data directory! "
                      "(reloading anyway)").format(cdir)
-            cmd = 'cd {0}; git pull'.format(settings.DATA_DIR, gitloc)
+            cmd = ['git', 'pull', ]
+            cwd = gdir
         else:
-            cmd = 'cd {0}; git clone {1}'.format(settings.DATA_DIR, gitloc)
-            msg += u'<pre>%s</pre>' % escape(os.popen(cmd).read())
+            cmd = ['git', 'clone', gitloc, ]
+            cwd = settings.DATA_DIR
+        cmd_output = escape(subprocess.check_output(cmd, cwd=os.path.abspath(cwd)))
+        msg += u'<pre>{0}</pre>'.format(cmd_output)
+
         if not os.path.exists(gdir):
             msg += _('Failed to clone repository to {0}').format(gdir)
             return msg
@@ -487,7 +493,7 @@ def post(self, request):
                                  page='courses_sysdashboard')
 
         courses = self.get_courses()
-        if action == _('add_course'):
+        if action == 'add_course':
             gitloc = request.POST.get('repo_location', '').strip().replace(' ', '').replace(';', '')
             datatable = self.make_datatable()
             self.msg += self.get_course_from_git(gitloc, datatable)
@@ -619,9 +625,14 @@ class GitLogs(TemplateView):
     template_name = 'sysadmin_dashboard_gitlogs.html'
 
     @method_decorator(login_required)
-    def get(self, request, course_id=None):
+    def get(self, request, *args, **kwargs):
         """Shows logs of imports that happened as a result of a git import"""
 
+        if 'course_id' in kwargs:
+            course_id = kwargs['course_id']
+        else:
+            course_id = None
+
         # Set mongodb defaults even if it isn't defined in settings
         mongo_db = {
             'host': 'localhost',

lms/djangoapps/dashboard/sysadmin_urls.py

@@ -1,6 +1,7 @@
 """
 Urls for sysadmin dashboard feature
 """
+# pylint: disable=E1120
 
 from django.conf.urls import patterns, url
 

lms/djangoapps/dashboard/tests/test_sysadmin.py

@@ -16,7 +16,7 @@
 from django.utils.translation import ugettext as _
 import mongoengine
 
-from courseware.roles import CourseStaffRole
+from courseware.roles import CourseStaffRole, GlobalStaff
 from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 from dashboard.models import CourseImportLog
 from dashboard.sysadmin import Users
@@ -52,8 +52,7 @@ def setUp(self):
     def _setstaff_login(self):
         """Makes the test user staff and logs them in"""
 
-        self.user.is_staff = True
-        self.user.save()
+        GlobalStaff().add_users(self.user)
         self.client.login(username=self.user.username, password='foo')
 
     def _add_edx4edx(self):