Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
e949c27
Add sysadmin dashboard - for seeting overview of system status, for
ichuang Aug 12, 2013
29929c6
add mongoengine to requirements
ichuang Aug 12, 2013
f55127b
Merge branch 'master' of github.com:edx/edx-platform into feature/ich…
ichuang Oct 8, 2013
c4f03f3
Merge branch 'master' of github.com:edx/edx-platform into feature/ich…
ichuang Oct 22, 2013
9b8b89a
This is a major revamp to make the sysadmin dashboard testable and
carsongee Oct 24, 2013
0c0b28c
After testing with mongo cluster with aws.py settings, there are a fe…
carsongee Oct 25, 2013
fc3baf7
Erroneous logging message removed
carsongee Oct 28, 2013
7deed40
More production/dev fixes
carsongee Oct 28, 2013
096d0ac
Added addtional import logger for debugging course imports
carsongee Oct 28, 2013
283b6de
Merge pull request #1490 from carsongee/feature/cg/sysadmin-dashboard…
ichuang Oct 29, 2013
57fe667
Added additional loggers for storing in mongo instead of just showing…
carsongee Oct 30, 2013
2c22969
This changes it to allow course staff to see their git import logs
carsongee Nov 5, 2013
19060dd
Merge pull request #1544 from carsongee/feature/cg/sysadmin-dashboard…
ichuang Nov 5, 2013
705f623
Code review based fixes:
carsongee Nov 14, 2013
aed34d7
Refactor of git_add_course.py from shell script to management command
carsongee Nov 14, 2013
9bc44c0
Refactor of sysadmin dashboard to use class views, and url includes
carsongee Nov 15, 2013
f2fdc7d
Class based view refactoring
carsongee Nov 19, 2013
5a038ed
Merge pull request #1708 from carsongee/feature/cg/sysadmin-dashboard…
ichuang Nov 19, 2013
a418834
Merge remote-tracking branch 'edx/master' into feature/ichuang/sysadm…
ichuang Nov 19, 2013
4bdbbba
Refactor to comply with changes added in PR #1575
carsongee Nov 19, 2013
f4502ca
Merge pull request #1717 from carsongee/feature/cg/sysadmin-dashboard…
ichuang Nov 19, 2013
b8f2949
Small UI glitch and settings check fixes
carsongee Nov 19, 2013
96cdbd9
Merge pull request #1720 from carsongee/feature/cg/sysadmin-dashboard…
ichuang Nov 19, 2013
d2d3e55
Several minor fixes/improvements, and the addition of a flag
carsongee Nov 20, 2013
f7f774a
Merge pull request #1732 from carsongee/feature/cg/sysadmin-dashboard…
ichuang Nov 20, 2013
5e062b2
sysadmin dashboard - Removal of popen, more streamlined git commands,…
carsongee Dec 4, 2013
898cd79
Merge branch 'feature/cg/sysadmin-dashboard-mongook' of github.com:ca…
carsongee Dec 4, 2013
d82bd74
Merge pull request #1843 from carsongee/feature/cg/sysadmin-dashboard…
ichuang Dec 4, 2013
c65a775
Merge branch 'master' of github.com:edx/edx-platform into feature/ich…
ichuang Dec 5, 2013
bb922c2
MITX_ and mitxmako removal from master refactor
carsongee Dec 5, 2013
9a02fdb
Merge pull request #1866 from carsongee/feature/cg/sysadmin-dashboard…
ichuang Dec 5, 2013
b117676
Bug fixes and refactors to sysadmin dashboard
carsongee Dec 11, 2013
d910d4a
Missing dash in ls argument
carsongee Dec 11, 2013
343637a
Merge pull request #1921 from carsongee/feature/cg/sysadmin-dashboard…
ichuang Dec 11, 2013
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions cms/djangoapps/contentstore/management/commands/import.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,16 @@ def handle(self, *args, **options):
course_dirs = args[1:]
else:
course_dirs = None
print("Importing. Data_dir={data}, course_dirs={courses}".format(
self.stdout.write("Importing. Data_dir={data}, course_dirs={courses}\n".format(
data=data_dir,
courses=course_dirs,
dis=do_import_static))
import_from_xml(modulestore('direct'), data_dir, course_dirs, load_error_modules=False,
try:
mstore = modulestore('direct')
except KeyError:
self.stdout.write('Unable to load direct modulestore, trying '
'default\n')
mstore = modulestore('default')

import_from_xml(mstore, data_dir, course_dirs, load_error_modules=False,
static_content_store=contentstore(), verbose=True, do_import_static=do_import_static)
1 change: 1 addition & 0 deletions lms/djangoapps/courseware/management/commands/import.py
Empty file.
Empty file.
222 changes: 222 additions & 0 deletions lms/djangoapps/dashboard/management/commands/git_add_course.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
"""
Script for importing courseware from git/xml into a mongo modulestore
"""

import os
import re
import datetime
import StringIO
import subprocess
import logging

from django.conf import settings
from django.core import management
from django.core.management.base import BaseCommand, CommandError
from django.utils.translation import ugettext as _
import mongoengine

from dashboard.models import CourseImportLog
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.xml import XMLModuleStore

log = logging.getLogger(__name__)

GIT_REPO_DIR = getattr(settings, 'GIT_REPO_DIR', '/opt/edx/course_repos')
GIT_IMPORT_STATIC = getattr(settings, 'GIT_IMPORT_STATIC', True)


def add_repo(repo, rdir_in):
"""This will add a git repo into the mongo modulestore"""
# pylint: disable=R0915

# Set defaults even if it isn't defined in settings
mongo_db = {
'host': 'localhost',
'user': '',
'password': '',
'db': 'xlog',
}

# Allow overrides
if hasattr(settings, 'MONGODB_LOG'):
for config_item in ['host', 'user', 'password', 'db', ]:
mongo_db[config_item] = settings.MONGODB_LOG.get(
config_item, mongo_db[config_item])

if not os.path.isdir(GIT_REPO_DIR):
log.critical(_("Path {0} doesn't exist, please create it, "
"or configure a different path with "
"GIT_REPO_DIR").format(GIT_REPO_DIR))
return -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raise an exception, rather than returning a status code


# pull from git
if not (repo.endswith('.git') or repo.startswith('http:') or
repo.startswith('https:') or repo.startswith('git:')):

log.error(_('Oops, not a git ssh url?'))
log.error(_('Expecting something like '
'git@github.com:mitocw/edx4edx_lite.git'))
return -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raise an exception, rather than returning a status code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all of these I'm intentionally not raising exceptions because of the two ways this gets called, and also because it was originally a shell script. As a result it made more sense to log and return a non-zero status. If I just raise exceptions each time, that means in sysadmin.py I have to catch all the various versions of exceptions in sysadmin.py instead of just printing the log messages to the user on the Web page, and generally letting them know it failed. This seemed simpler and more versatile for the two access paths to the script.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see.


if rdir_in:
rdir = rdir_in
rdir = os.path.basename(rdir)
else:
rdir = repo.rsplit('/', 1)[-1].rsplit('.git', 1)[0]

log.debug('rdir = {0}'.format(rdir))

rdirp = '{0}/{1}'.format(GIT_REPO_DIR, rdir)
if os.path.exists(rdirp):
log.info(_('directory already exists, doing a git pull instead '
'of git clone'))
cmd = ['git', 'pull', ]
cwd = '{0}/{1}'.format(GIT_REPO_DIR, rdir)
else:
cmd = ['git', 'clone', repo, ]
cwd = GIT_REPO_DIR

log.debug(cmd)
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, it's better to raise a relevant exception, rather than returning a status code like this.


# get commit id
cmd = ['git', 'log', '-1', '--format=%H', ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd probably be better off using GitPython rather than using subprocess to call the git binary. We already have GitPython in our requirements files, so it should already be installed.

commit_id = subprocess.check_output(cmd, cwd=rdirp)

ret_git += _('\nCommit ID: {0}').format(commit_id)

# get branch
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_log_handler = logging.StreamHandler(output)
import_log_handler.setLevel(logging.DEBUG)

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you iterate over the log names, rather than the log objects, then you can put the logging.getLogger call inside the loop as well.

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:
log.exception(_('Unable to run import command.'))
return -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just allow this exception to percolate, rather than catching it and returning a status code. Or, you could make a more meaningful exception class an raise that.

except NotImplementedError:
log.exception(_('The underlying module store does not support import.'))
return -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above: raise, rather than returning -1


ret_import = output.getvalue()

# Remove handler hijacks
for logger in loggers:
logger.setLevel(logger.old_level)
logger.removeHandler(import_log_handler)

course_id = 'unknown'
location = 'unknown'

# extract course ID from output of import-command-run and make symlink
# this is needed in order for custom course scripts to work
match = re.search('(?ms)===> IMPORTING course to location ([^ \n]+)',
ret_import)
if match:
location = match.group(1).strip()
log.debug('location = {0}'.format(location))
course_id = location.replace('i4x://', '').replace(
'/course/', '/').split('\n')[0].strip()

cdir = '{0}/{1}'.format(GIT_REPO_DIR, course_id.split('/')[1])
log.debug(_('Studio course dir = {0}').format(cdir))

if os.path.exists(cdir) and not os.path.islink(cdir):
log.debug(_(' -> exists, but is not symlink'))
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 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://{user}:{password}@{host}/{db}'.format(**mongo_db)

try:
if mongo_db['user'] and mongo_db['password']:
mdb = mongoengine.connect(mongo_db['db'], host=mongouri)
else:
mdb = mongoengine.connect(mongo_db['db'], host=mongo_db['host'])
except mongoengine.connection.ConnectionError:
log.exception(_('Unable to connect to mongodb to save log, please '
'check MONGODB_LOG settings'))
return -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raise, rather than returning.

cil = CourseImportLog(
course_id=course_id,
location=location,
repo_dir=rdir,
created=datetime.datetime.now(),
import_log=ret_import,
git_log=ret_git,
)
cil.save()

log.debug(_('saved CourseImportLog for {0}').format(cil.course_id))
mdb.disconnect()
return 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once everywhere that returns -1 raises instead, this can go away.



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')

def handle(self, *args, **options):
"""Check inputs and run the command"""

if isinstance(modulestore, XMLModuleStore):
raise CommandError(_('This script requires a mongo module store'))

if len(args) < 1:
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 '
'arguments.'))

rdir_arg = None

if len(args) > 1:
rdir_arg = args[1]

if add_repo(args[0], rdir_arg) != 0:
raise CommandError(_('Repo was not added, check log output '
'for details'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than call and then check a response code, use try ... except ... to provide more context to errors.

18 changes: 17 additions & 1 deletion lms/djangoapps/dashboard/models.py
Original file line number Diff line number Diff line change
@@ -1 +1,17 @@
# Create your models here.
"""Models for dashboard application"""

import mongoengine


class CourseImportLog(mongoengine.Document):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ship may have sailed, since you guys already have logs in mongo, but why not just use Django models and log into the relational database? This doesn't seem like a place where it's a real benefit to use a document store, rather than just using the relational database that is already configured.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ichuang do you remember? Operationally it has gotten quite large, and each log message is quite large, which for us is easier to manage in mongo, but I don't know specifically why it was selected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logs get very large, and are thus more suited to a document db, rather than an sql one. I'd like to leave it as using mongo.

"""Mongoengine model for git log"""
# pylint: disable=R0924

course_id = mongoengine.StringField(max_length=128)
location = mongoengine.StringField(max_length=168)
import_log = mongoengine.StringField(max_length=20 * 65535)
git_log = mongoengine.StringField(max_length=65535)
repo_dir = mongoengine.StringField(max_length=128)
created = mongoengine.DateTimeField()
meta = {'indexes': ['course_id', 'created'],
'allow_inheritance': False}
Loading