From 9d55c88c76846d0bbf6501bb4d67f262e1529c6b Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Thu, 23 Jan 2014 13:17:50 -0500 Subject: [PATCH] Add option for importing a course from a named branch of a git repo --- lms/djangoapps/dashboard/git_import.py | 62 ++++++++- .../management/commands/git_add_course.py | 15 ++- .../commands/tests/test_git_add_course.py | 118 +++++++++++++++--- lms/djangoapps/dashboard/sysadmin.py | 39 +++--- .../dashboard/tests/test_sysadmin.py | 28 ++++- lms/templates/sysadmin_dashboard.html | 10 +- 6 files changed, 230 insertions(+), 42 deletions(-) diff --git a/lms/djangoapps/dashboard/git_import.py b/lms/djangoapps/dashboard/git_import.py index 812dc34796..7db862bc2a 100644 --- a/lms/djangoapps/dashboard/git_import.py +++ b/lms/djangoapps/dashboard/git_import.py @@ -39,7 +39,9 @@ class GitImportError(Exception): CANNOT_PULL = _('git clone or pull failed!') XML_IMPORT_FAILED = _('Unable to run import command.') UNSUPPORTED_STORE = _('The underlying module store does not support import.') - + REMOTE_BRANCH_MISSING = _('The specified remote branch is not available.') + CANNOT_BRANCH = _('Unable to switch to specified branch. Please check ' + 'your branch name.') def cmd_log(cmd, cwd): """ @@ -54,7 +56,60 @@ def cmd_log(cmd, cwd): return output -def add_repo(repo, rdir_in): +def switch_branch(branch, rdir): + """ + This will determine how to change the branch of the repo, and then + use the appropriate git commands to do so. + + Raises an appropriate GitImportError exception if there is any issues with changing + branches. + """ + # Get the latest remote + try: + cmd_log(['git', 'fetch', ], rdir) + except subprocess.CalledProcessError as ex: + log.exception('Unable to fetch remote: %r', ex.output) + raise GitImportError(GitImportError.CANNOT_BRANCH) + + # Check if the branch is available from the remote. + cmd = ['git', 'ls-remote', 'origin', '-h', 'refs/heads/{0}'.format(branch), ] + try: + output = cmd_log(cmd, rdir) + except subprocess.CalledProcessError as ex: + log.exception('Getting a list of remote branches failed: %r', ex.output) + raise GitImportError(GitImportError.CANNOT_BRANCH) + if not branch in output: + raise GitImportError(GitImportError.REMOTE_BRANCH_MISSING) + # Check it the remote branch has already been made locally + cmd = ['git', 'branch', '-a', ] + try: + output = cmd_log(cmd, rdir) + except subprocess.CalledProcessError as ex: + log.exception('Getting a list of local branches failed: %r', ex.output) + raise GitImportError(GitImportError.CANNOT_BRANCH) + branches = [] + for line in output.split('\n'): + branches.append(line.strip()) + + if branch not in branches: + # Checkout with -b since it is remote only + cmd = ['git', 'checkout', '--force', '--track', + '-b', branch, 'origin/{0}'.format(branch), ] + try: + cmd_log(cmd, rdir) + except subprocess.CalledProcessError as ex: + log.exception('Unable to checkout remote branch: %r', ex.output) + raise GitImportError(GitImportError.CANNOT_BRANCH) + # Go ahead and reset hard to the newest version of the branch now that we know + # it is local. + try: + cmd_log(['git', 'reset', '--hard', 'origin/{0}'.format(branch), ], rdir) + except subprocess.CalledProcessError as ex: + log.exception('Unable to reset to branch: %r', ex.output) + raise GitImportError(GitImportError.CANNOT_BRANCH) + + +def add_repo(repo, rdir_in, branch): """This will add a git repo into the mongo modulestore""" # pylint: disable=R0915 @@ -102,6 +157,9 @@ def add_repo(repo, rdir_in): log.exception('Error running git pull: %r', ex.output) raise GitImportError(GitImportError.CANNOT_PULL) + if branch: + switch_branch(branch, rdirp) + # get commit id cmd = ['git', 'log', '-1', '--format=%H', ] try: diff --git a/lms/djangoapps/dashboard/management/commands/git_add_course.py b/lms/djangoapps/dashboard/management/commands/git_add_course.py index 4a184e4cd2..af5c05c176 100644 --- a/lms/djangoapps/dashboard/management/commands/git_add_course.py +++ b/lms/djangoapps/dashboard/management/commands/git_add_course.py @@ -25,8 +25,10 @@ class Command(BaseCommand): Pull a git repo and import into the mongo based content database. """ - help = _('Import the specified git repository into the ' - 'modulestore and directory') + help = ('Usage: ' + 'git_add_course repository_url [directory to check out into] [repository_branch] ' + '\n{0}'.format(_('Import the specified git repository and optional branch into the ' + 'modulestore and optionally specified directory.'))) def handle(self, *args, **options): """Check inputs and run the command""" @@ -38,16 +40,19 @@ class Command(BaseCommand): raise CommandError('This script requires at least one argument, ' 'the git URL') - if len(args) > 2: - raise CommandError('This script requires no more than two ' + if len(args) > 3: + raise CommandError('This script requires no more than three ' 'arguments') rdir_arg = None + branch = None if len(args) > 1: rdir_arg = args[1] + if len(args) > 2: + branch = args[2] try: - dashboard.git_import.add_repo(args[0], rdir_arg) + dashboard.git_import.add_repo(args[0], rdir_arg, branch) except GitImportError as ex: raise CommandError(str(ex)) diff --git a/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py b/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py index fd7c81dd56..9d04f48b5d 100644 --- a/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py +++ b/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py @@ -2,11 +2,12 @@ Provide tests for git_add_course management command. """ -import unittest +import logging import os import shutil import StringIO import subprocess +import unittest from django.conf import settings from django.core.management import call_command @@ -14,6 +15,9 @@ from django.core.management.base import CommandError from django.test.utils import override_settings from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE +from xmodule.contentstore.django import contentstore +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.store_utilities import delete_course from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase import dashboard.git_import as git_import from dashboard.git_import import GitImportError @@ -39,6 +43,9 @@ class TestGitAddCourse(ModuleStoreTestCase): """ TEST_REPO = 'https://github.com/mitocw/edx4edx_lite.git' + TEST_COURSE = 'MITx/edx4edx/edx4edx' + TEST_BRANCH = 'testing_do_not_delete' + TEST_BRANCH_COURSE = 'MITx/edx4edx_branch/edx4edx' def assertCommandFailureRegexp(self, regex, *args): """ @@ -56,16 +63,15 @@ class TestGitAddCourse(ModuleStoreTestCase): self.assertCommandFailureRegexp( 'This script requires at least one argument, the git URL') self.assertCommandFailureRegexp( - 'This script requires no more than two arguments', - 'blah', 'blah', 'blah') + 'This script requires no more than three arguments', + 'blah', 'blah', 'blah', 'blah') self.assertCommandFailureRegexp( 'Repo was not added, check log output for details', 'blah') # Test successful import from command - try: + if not os.path.isdir(getattr(settings, 'GIT_REPO_DIR')): os.mkdir(getattr(settings, 'GIT_REPO_DIR')) - except OSError: - pass + self.addCleanup(shutil.rmtree, getattr(settings, 'GIT_REPO_DIR')) # Make a course dir that will be replaced with a symlink # while we are at it. @@ -74,24 +80,28 @@ class TestGitAddCourse(ModuleStoreTestCase): call_command('git_add_course', self.TEST_REPO, getattr(settings, 'GIT_REPO_DIR') / 'edx4edx_lite') - if os.path.isdir(getattr(settings, 'GIT_REPO_DIR')): - shutil.rmtree(getattr(settings, 'GIT_REPO_DIR')) + + # Test with all three args (branch) + call_command('git_add_course', self.TEST_REPO, + getattr(settings, 'GIT_REPO_DIR') / 'edx4edx_lite', + self.TEST_BRANCH) + def test_add_repo(self): """ Various exit path tests for test_add_repo """ with self.assertRaisesRegexp(GitImportError, GitImportError.NO_DIR): - git_import.add_repo(self.TEST_REPO, None) + git_import.add_repo(self.TEST_REPO, None, None) os.mkdir(getattr(settings, 'GIT_REPO_DIR')) self.addCleanup(shutil.rmtree, getattr(settings, 'GIT_REPO_DIR')) with self.assertRaisesRegexp(GitImportError, GitImportError.URL_BAD): - git_import.add_repo('foo', None) + git_import.add_repo('foo', None, None) with self.assertRaisesRegexp(GitImportError, GitImportError.CANNOT_PULL): - git_import.add_repo('file:///foobar.git', None) + git_import.add_repo('file:///foobar.git', None, None) # Test git repo that exists, but is "broken" bare_repo = os.path.abspath('{0}/{1}'.format(settings.TEST_ROOT, 'bare.git')) @@ -101,7 +111,7 @@ class TestGitAddCourse(ModuleStoreTestCase): cwd=bare_repo) with self.assertRaisesRegexp(GitImportError, GitImportError.BAD_REPO): - git_import.add_repo('file://{0}'.format(bare_repo), None) + git_import.add_repo('file://{0}'.format(bare_repo), None, None) def test_detached_repo(self): """ @@ -114,9 +124,89 @@ class TestGitAddCourse(ModuleStoreTestCase): except OSError: pass self.addCleanup(shutil.rmtree, repo_dir) - git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite') + git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite', None) subprocess.check_output(['git', 'checkout', 'HEAD~2', ], stderr=subprocess.STDOUT, cwd=repo_dir / 'edx4edx_lite') with self.assertRaisesRegexp(GitImportError, GitImportError.CANNOT_PULL): - git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite') + git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite', None) + + def test_branching(self): + """ + Exercise branching code of import + """ + repo_dir = getattr(settings, 'GIT_REPO_DIR') + # Test successful import from command + if not os.path.isdir(repo_dir): + os.mkdir(repo_dir) + self.addCleanup(shutil.rmtree, repo_dir) + + # Checkout non existent branch + with self.assertRaisesRegexp(GitImportError, GitImportError.REMOTE_BRANCH_MISSING): + git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite', 'asdfasdfasdf') + + # Checkout new branch + git_import.add_repo(self.TEST_REPO, + repo_dir / 'edx4edx_lite', + self.TEST_BRANCH) + def_ms = modulestore() + # Validate that it is different than master + self.assertIsNotNone(def_ms.get_course(self.TEST_BRANCH_COURSE)) + + # Delete to test branching back to master + delete_course(def_ms, contentstore(), + def_ms.get_course(self.TEST_BRANCH_COURSE).location, + True) + self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE)) + git_import.add_repo(self.TEST_REPO, + repo_dir / 'edx4edx_lite', + 'master') + self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE)) + self.assertIsNotNone(def_ms.get_course(self.TEST_COURSE)) + + def test_branch_exceptions(self): + """ + This wil create conditions to exercise bad paths in the switch_branch function. + """ + # create bare repo that we can mess with and attempt an import + bare_repo = os.path.abspath('{0}/{1}'.format(settings.TEST_ROOT, 'bare.git')) + os.mkdir(bare_repo) + self.addCleanup(shutil.rmtree, bare_repo) + subprocess.check_output(['git', '--bare', 'init', ], stderr=subprocess.STDOUT, + cwd=bare_repo) + + # Build repo dir + repo_dir = getattr(settings, 'GIT_REPO_DIR') + if not os.path.isdir(repo_dir): + os.mkdir(repo_dir) + self.addCleanup(shutil.rmtree, repo_dir) + + rdir = '{0}/bare'.format(repo_dir) + with self.assertRaisesRegexp(GitImportError, GitImportError.BAD_REPO): + git_import.add_repo('file://{0}'.format(bare_repo), None, None) + + # Get logger for checking strings in logs + output = StringIO.StringIO() + test_log_handler = logging.StreamHandler(output) + test_log_handler.setLevel(logging.DEBUG) + glog = git_import.log + glog.addHandler(test_log_handler) + + # Move remote so fetch fails + shutil.move(bare_repo, '{0}/not_bare.git'.format(settings.TEST_ROOT)) + try: + git_import.switch_branch('master', rdir) + except GitImportError: + self.assertIn('Unable to fetch remote', output.getvalue()) + shutil.move('{0}/not_bare.git'.format(settings.TEST_ROOT), bare_repo) + output.truncate(0) + + # Replace origin with a different remote + subprocess.check_output( + ['git', 'remote', 'rename', 'origin', 'blah', ], + stderr=subprocess.STDOUT, cwd=rdir + ) + try: + git_import.switch_branch('master', rdir) + except GitImportError: + self.assertIn('Getting a list of remote branches failed', output.getvalue()) diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index 81c0e2824e..67f81cfe6f 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/lms/djangoapps/dashboard/sysadmin.py @@ -272,7 +272,7 @@ class Users(SysadminDashboardView): 'msg': self.msg, 'djangopid': os.getpid(), 'modeflag': {'users': 'active-section'}, - 'mitx_version': getattr(settings, 'VERSION_STRING', ''), + 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), } return render_to_response(self.template_name, context) @@ -316,7 +316,7 @@ class Users(SysadminDashboardView): 'msg': self.msg, 'djangopid': os.getpid(), 'modeflag': {'users': 'active-section'}, - 'mitx_version': getattr(settings, 'VERSION_STRING', ''), + 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), } return render_to_response(self.template_name, context) @@ -348,7 +348,7 @@ class Courses(SysadminDashboardView): return info - def get_course_from_git(self, gitloc, datatable): + def get_course_from_git(self, gitloc, branch, datatable): """This downloads and runs the checks for importing a course in git""" if not (gitloc.endswith('.git') or gitloc.startswith('http:') or @@ -357,11 +357,11 @@ class Courses(SysadminDashboardView): "and be a valid url") if self.is_using_mongo: - return self.import_mongo_course(gitloc) + return self.import_mongo_course(gitloc, branch) - return self.import_xml_course(gitloc, datatable) + return self.import_xml_course(gitloc, branch, datatable) - def import_mongo_course(self, gitloc): + def import_mongo_course(self, gitloc, branch): """ Imports course using management command and captures logging output at debug level for display in template @@ -390,7 +390,7 @@ class Courses(SysadminDashboardView): error_msg = '' try: - git_import.add_repo(gitloc, None) + git_import.add_repo(gitloc, None, branch) except GitImportError as ex: error_msg = str(ex) ret = output.getvalue() @@ -411,7 +411,7 @@ class Courses(SysadminDashboardView): msg += "
{0}
".format(escape(ret)) return msg - def import_xml_course(self, gitloc, datatable): + def import_xml_course(self, gitloc, branch, datatable): """Imports a git course into the XMLModuleStore""" msg = u'' @@ -436,13 +436,23 @@ class Courses(SysadminDashboardView): cmd_output = escape( subprocess.check_output(cmd, stderr=subprocess.STDOUT, cwd=cwd) ) - except subprocess.CalledProcessError: - return _('Unable to clone or pull repository. Please check your url.') + except subprocess.CalledProcessError as ex: + log.exception('Git pull or clone output was: %r', ex.output) + return _('Unable to clone or pull repository. Please check ' + 'your url. Output was: {0!r}'.format(ex.output)) msg += u'
{0}
'.format(cmd_output) if not os.path.exists(gdir): msg += _('Failed to clone repository to {0}').format(gdir) return msg + # Change branch if specified + if branch: + try: + git_import.switch_branch(branch, gdir) + except GitImportError as ex: + return str(ex) + msg += u'

{0}: {1}

'.format(_('Successfully switched to branch'), branch) + self.def_ms.try_load_course(os.path.abspath(gdir)) errlog = self.def_ms.errored_courses.get(cdir, '') if errlog: @@ -494,7 +504,7 @@ class Courses(SysadminDashboardView): 'msg': self.msg, 'djangopid': os.getpid(), 'modeflag': {'courses': 'active-section'}, - 'mitx_version': getattr(settings, 'VERSION_STRING', ''), + 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), } return render_to_response(self.template_name, context) @@ -511,8 +521,9 @@ class Courses(SysadminDashboardView): courses = self.get_courses() if action == 'add_course': gitloc = request.POST.get('repo_location', '').strip().replace(' ', '').replace(';', '') + branch = request.POST.get('repo_branch', '').strip().replace(' ', '').replace(';', '') datatable = self.make_datatable() - self.msg += self.get_course_from_git(gitloc, datatable) + self.msg += self.get_course_from_git(gitloc, branch, datatable) elif action == 'del_course': course_id = request.POST.get('course_id', '').strip() @@ -563,7 +574,7 @@ class Courses(SysadminDashboardView): 'msg': self.msg, 'djangopid': os.getpid(), 'modeflag': {'courses': 'active-section'}, - 'mitx_version': getattr(settings, 'VERSION_STRING', ''), + 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), } return render_to_response(self.template_name, context) @@ -602,7 +613,7 @@ class Staffing(SysadminDashboardView): 'msg': self.msg, 'djangopid': os.getpid(), 'modeflag': {'staffing': 'active-section'}, - 'mitx_version': getattr(settings, 'VERSION_STRING', ''), + 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), } return render_to_response(self.template_name, context) diff --git a/lms/djangoapps/dashboard/tests/test_sysadmin.py b/lms/djangoapps/dashboard/tests/test_sysadmin.py index fb20c4f345..19f97ff5e0 100644 --- a/lms/djangoapps/dashboard/tests/test_sysadmin.py +++ b/lms/djangoapps/dashboard/tests/test_sysadmin.py @@ -45,6 +45,10 @@ class SysadminBaseTestCase(ModuleStoreTestCase): Base class with common methods used in XML and Mongo tests """ + TEST_REPO = 'https://github.com/mitocw/edx4edx_lite.git' + TEST_BRANCH = 'testing_do_not_delete' + TEST_BRANCH_COURSE = 'MITx/edx4edx_branch/edx4edx' + def setUp(self): """Setup test case by adding primary user.""" super(SysadminBaseTestCase, self).setUp() @@ -58,11 +62,12 @@ class SysadminBaseTestCase(ModuleStoreTestCase): GlobalStaff().add_users(self.user) self.client.login(username=self.user.username, password='foo') - def _add_edx4edx(self): + def _add_edx4edx(self, branch=None): """Adds the edx4edx sample course""" - return self.client.post(reverse('sysadmin_courses'), { - 'repo_location': 'https://github.com/mitocw/edx4edx_lite.git', - 'action': 'add_course', }) + post_dict = {'repo_location': self.TEST_REPO, 'action': 'add_course', } + if branch: + post_dict['repo_branch'] = branch + return self.client.post(reverse('sysadmin_courses'), post_dict) def _rm_edx4edx(self): """Deletes the sample course from the XML store""" @@ -301,11 +306,24 @@ class TestSysadmin(SysadminBaseTestCase): self.assertIsNotNone(course) # Delete a course - response = self._rm_edx4edx() + self._rm_edx4edx() course = def_ms.courses.get('{0}/edx4edx_lite'.format( os.path.abspath(settings.DATA_DIR)), None) self.assertIsNone(course) + # Load a bad git branch + response = self._add_edx4edx('asdfasdfasdf') + self.assertIn(GitImportError.REMOTE_BRANCH_MISSING, + response.content.decode('utf-8')) + + # Load a course from a git branch + self._add_edx4edx(self.TEST_BRANCH) + course = def_ms.courses.get('{0}/edx4edx_lite'.format( + os.path.abspath(settings.DATA_DIR)), None) + self.assertIsNotNone(course) + self.assertIn(self.TEST_BRANCH_COURSE, course.location.course_id) + self._rm_edx4edx() + # Try and delete a non-existent course response = self.client.post(reverse('sysadmin_courses'), {'course_id': 'foobar/foo/blah', diff --git a/lms/templates/sysadmin_dashboard.html b/lms/templates/sysadmin_dashboard.html index 21dcf4c869..dcca467e3a 100644 --- a/lms/templates/sysadmin_dashboard.html +++ b/lms/templates/sysadmin_dashboard.html @@ -126,10 +126,16 @@ textarea {
@@ -201,6 +207,6 @@ textarea {
${_('Django PID')}: ${djangopid} - | ${_('Platform Version')}: ${mitx_version}
+ | ${_('Platform Version')}: ${edx_platform_version}