From 59defd33b687d73da933aabdca6b7b34e6d75a8b Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Fri, 27 Jul 2012 16:06:00 -0400 Subject: [PATCH] Formatting and little bugfixes * add course and org to full/course.xml * fix error path in clean_xml script * commit rakefile change needed by c5334f150c --- common/lib/xmodule/xmodule/capa_module.py | 6 +++-- common/lib/xmodule/xmodule/modulestore/xml.py | 14 +++++----- common/lib/xmodule/xmodule/video_module.py | 2 +- common/test/data/full/course.xml | 2 +- .../management/commands/clean_xml.py | 2 +- lms/djangoapps/courseware/module_render.py | 14 ++++++---- lms/envs/common.py | 26 +++++++++---------- rakefile | 5 ++-- 8 files changed, 40 insertions(+), 31 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 735f664b0a..02987822db 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -530,11 +530,13 @@ class CapaModule(XModule): self.lcp.do_reset() if self.rerandomize == "always": - # reset random number generator seed (note the self.lcp.get_state() in next line) + # reset random number generator seed (note the self.lcp.get_state() + # in next line) self.lcp.seed = None self.lcp = LoncapaProblem(self.definition['data'], - self.location.html_id(), self.lcp.get_state(), system=self.system) + self.location.html_id(), self.lcp.get_state(), + system=self.system) event_info['new_state'] = self.lcp.get_state() self.system.track_function('reset_problem', event_info) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index cfeac97780..7dd6868f78 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -74,7 +74,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): etree.tostring(xml_data), self, org, course, xmlstore.default_class) - log.debug('==> importing module location %s' % repr(module.location)) + #log.debug('==> importing module location %s' % repr(module.location)) module.metadata['data_dir'] = course_dir xmlstore.modules[module.location] = module @@ -130,13 +130,14 @@ class XMLModuleStore(ModuleStore): self.default_class = None else: module_path, _, class_name = default_class.rpartition('.') - log.debug('module_path = %s' % module_path) + #log.debug('module_path = %s' % module_path) class_ = getattr(import_module(module_path), class_name) self.default_class = class_ - # TODO (cpennington): We need a better way of selecting specific sets of debug messages to enable. These were drowning out important messages - log.debug('XMLModuleStore: eager=%s, data_dir = %s' % (eager, self.data_dir)) - log.debug('default_class = %s' % self.default_class) + # TODO (cpennington): We need a better way of selecting specific sets of + # debug messages to enable. These were drowning out important messages + #log.debug('XMLModuleStore: eager=%s, data_dir = %s' % (eager, self.data_dir)) + #log.debug('default_class = %s' % self.default_class) # If we are specifically asked for missing courses, that should # be an error. If we are asked for "all" courses, find the ones @@ -162,6 +163,7 @@ class XMLModuleStore(ModuleStore): returns a CourseDescriptor for the course """ + log.debug('========> Starting course import from {0}'.format(course_dir)) with open(self.data_dir / course_dir / "course.xml") as course_file: @@ -192,7 +194,7 @@ class XMLModuleStore(ModuleStore): self.error_handler) course_descriptor = system.process_xml(etree.tostring(course_data)) - log.debug('========> Done with course import') + log.debug('========> Done with course import from {0}'.format(course_dir)) return course_descriptor def get_item(self, location, depth=0): diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index a8f6db9c27..da10e4bc91 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -60,7 +60,7 @@ class VideoModule(XModule): return None def get_instance_state(self): - log.debug(u"STATE POSITION {0}".format(self.position)) + #log.debug(u"STATE POSITION {0}".format(self.position)) return json.dumps({'position': self.position}) def video_list(self): diff --git a/common/test/data/full/course.xml b/common/test/data/full/course.xml index 607d22672c..c365e68cc1 100644 --- a/common/test/data/full/course.xml +++ b/common/test/data/full/course.xml @@ -1 +1 @@ - + diff --git a/lms/djangoapps/courseware/management/commands/clean_xml.py b/lms/djangoapps/courseware/management/commands/clean_xml.py index bd0c2b21bb..7523fd8373 100644 --- a/lms/djangoapps/courseware/management/commands/clean_xml.py +++ b/lms/djangoapps/courseware/management/commands/clean_xml.py @@ -104,7 +104,7 @@ def import_with_checks(course_dir, verbose=True): if n != 1: print 'ERROR: Expect exactly 1 course. Loaded {n}: {lst}'.format( n=n, lst=courses) - return + return (False, None) course = courses[0] diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 01e46b8def..bf7d1f4c51 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -119,16 +119,18 @@ def get_module(user, request, location, student_module_cache, position=None): instance_module is a StudentModule specific to this module for this student, or None if this is an anonymous user shared_module is a StudentModule specific to all modules with the same - 'shared_state_key' attribute, or None if the module doesn't elect to + 'shared_state_key' attribute, or None if the module does not elect to share state ''' descriptor = modulestore().get_item(location) - instance_module = student_module_cache.lookup(descriptor.category, descriptor.location.url()) + instance_module = student_module_cache.lookup(descriptor.category, + descriptor.location.url()) shared_state_key = getattr(descriptor, 'shared_state_key', None) if shared_state_key is not None: - shared_module = student_module_cache.lookup(descriptor.category, shared_state_key) + shared_module = student_module_cache.lookup(descriptor.category, + shared_state_key) else: shared_module = None @@ -138,10 +140,12 @@ def get_module(user, request, location, student_module_cache, position=None): # TODO (vshnayder): fix hardcoded urls (use reverse) # Setup system context for module instance ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/' - xqueue_callback_url = settings.MITX_ROOT_URL + '/xqueue/' + str(user.id) + '/' + descriptor.location.url() + '/' + xqueue_callback_url = (settings.MITX_ROOT_URL + '/xqueue/' + + str(user.id) + '/' + descriptor.location.url() + '/') def _get_module(location): - (module, _, _, _) = get_module(user, request, location, student_module_cache, position) + (module, _, _, _) = get_module(user, request, location, + student_module_cache, position) return module # TODO (cpennington): When modules are shared between courses, the static diff --git a/lms/envs/common.py b/lms/envs/common.py index c071e61bb4..6d46f1cf9d 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -6,16 +6,16 @@ MITX_FEATURES[...]. Modules that extend this one can change the feature configuration in an environment specific config file and re-calculate those values. -We should make a method that calls all these config methods so that you just +We should make a method that calls all these config methods so that you just make one call at the end of your site-specific dev file to reset all the dependent variables (like INSTALLED_APPS) for you. Longer TODO: -1. Right now our treatment of static content in general and in particular +1. Right now our treatment of static content in general and in particular course-specific static content is haphazard. 2. We should have a more disciplined approach to feature flagging, even if it just means that we stick them in a dict called MITX_FEATURES. -3. We need to handle configuration for multiple courses. This could be as +3. We need to handle configuration for multiple courses. This could be as multiple sites, but we do need a way to map their data assets. """ import sys @@ -42,7 +42,7 @@ MITX_FEATURES = { 'SAMPLE' : False, 'USE_DJANGO_PIPELINE' : True, 'DISPLAY_HISTOGRAMS_TO_STAFF' : True, - 'REROUTE_ACTIVATION_EMAIL' : False, # nonempty string = address for all activation emails + 'REROUTE_ACTIVATION_EMAIL' : False, # nonempty string = address for all activation emails 'DEBUG_LEVEL' : 0, # 0 = lowest level, least verbose, 255 = max level, most verbose ## DO NOT SET TO True IN THIS FILE @@ -85,7 +85,7 @@ MAKO_TEMPLATES['main'] = [PROJECT_ROOT / 'templates', COMMON_ROOT / 'lib' / 'capa' / 'capa' / 'templates', COMMON_ROOT / 'djangoapps' / 'pipeline_mako' / 'templates'] -# This is where Django Template lookup is defined. There are a few of these +# This is where Django Template lookup is defined. There are a few of these # still left lying around. TEMPLATE_DIRS = ( PROJECT_ROOT / "templates", @@ -103,8 +103,8 @@ TEMPLATE_CONTEXT_PROCESSORS = ( ) -# FIXME: -# We should have separate S3 staged URLs in case we need to make changes to +# FIXME: +# We should have separate S3 staged URLs in case we need to make changes to # these assets and test them. LIB_URL = '/static/js/' @@ -120,7 +120,7 @@ STATIC_GRAB = False DEV_CONTENT = True # FIXME: Should we be doing this truncation? -TRACK_MAX_EVENT = 10000 +TRACK_MAX_EVENT = 10000 DEBUG_TRACK_LOG = False MITX_ROOT_URL = '' @@ -129,7 +129,7 @@ COURSE_NAME = "6.002_Spring_2012" COURSE_NUMBER = "6.002x" COURSE_TITLE = "Circuits and Electronics" -### Dark code. Should be enabled in local settings for devel. +### Dark code. Should be enabled in local settings for devel. ENABLE_MULTICOURSE = False # set to False to disable multicourse display (see lib.util.views.mitxhome) QUICKEDIT = False @@ -211,9 +211,9 @@ USE_L10N = True MESSAGE_STORAGE = 'django.contrib.messages.storage.session.SessionStorage' #################################### AWS ####################################### -# S3BotoStorage insists on a timeout for uploaded assets. We should make it +# S3BotoStorage insists on a timeout for uploaded assets. We should make it # permanent instead, but rather than trying to figure out exactly where that -# setting is, I'm just bumping the expiration time to something absurd (100 +# setting is, I'm just bumping the expiration time to something absurd (100 # years). This is only used if DEFAULT_FILE_STORAGE is overriden to use S3 # in the global settings.py AWS_QUERYSTRING_EXPIRE = 10 * 365 * 24 * 60 * 60 # 10 years @@ -279,7 +279,7 @@ MIDDLEWARE_CLASSES = ( # Instead of AuthenticationMiddleware, we use a cached backed version #'django.contrib.auth.middleware.AuthenticationMiddleware', 'cache_toolbox.middleware.CacheBackedAuthenticationMiddleware', - + 'django.contrib.messages.middleware.MessageMiddleware', 'track.middleware.TrackMiddleware', 'mitxmako.middleware.MakoMiddleware', @@ -502,7 +502,7 @@ INSTALLED_APPS = ( # For testing 'django_jasmine', - # For Askbot + # For Askbot 'django.contrib.sitemaps', 'django.contrib.admin', 'django_countries', diff --git a/rakefile b/rakefile index fd23b9643a..d69f7329bb 100644 --- a/rakefile +++ b/rakefile @@ -51,6 +51,7 @@ default_options = { task :predjango do sh("find . -type f -name *.pyc -delete") sh('pip install -e common/lib/xmodule') + sh('git submodule update --init') end [:lms, :cms, :common].each do |system| @@ -150,14 +151,14 @@ end task :package do FileUtils.mkdir_p(BUILD_DIR) - + Dir.chdir(BUILD_DIR) do afterremove = Tempfile.new('afterremove') afterremove.write <<-AFTERREMOVE.gsub(/^\s*/, '') #! /bin/bash set -e set -x - + # to be a little safer this rm is executed # as the makeitso user