diff --git a/lms/djangoapps/dashboard/management/commands/git_add_course.py b/lms/djangoapps/dashboard/management/commands/git_add_course.py index 9e6737bafe37..f588920e303d 100644 --- a/lms/djangoapps/dashboard/management/commands/git_add_course.py +++ b/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,12 +71,15 @@ 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)): @@ -83,40 +87,37 @@ def add_repo(repo, rdir_in): 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, diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index d301e4fdf3a4..c66913920a10 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/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"

{0} {1}

".format( _('Added course from'), gitloc) msg += _("
{0}
").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'
%s
' % 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'
{0}
'.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', diff --git a/lms/djangoapps/dashboard/sysadmin_urls.py b/lms/djangoapps/dashboard/sysadmin_urls.py index 489cf877cf0e..e543537415e7 100644 --- a/lms/djangoapps/dashboard/sysadmin_urls.py +++ b/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 diff --git a/lms/djangoapps/dashboard/tests/test_sysadmin.py b/lms/djangoapps/dashboard/tests/test_sysadmin.py index 899befffe03b..95bc5bbb997b 100644 --- a/lms/djangoapps/dashboard/tests/test_sysadmin.py +++ b/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):