Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Performance enhancements for django-admin commands to delete and clone course #570

Merged
merged 3 commits into from
Aug 6, 2013
Merged

Performance enhancements for django-admin commands to delete and clone course #570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f57b02c
add a metadata cache and re-compute ignore when deleting a course to …
Aug 3, 2013
9f07507
add same optimizations to the clone course command
Aug 3, 2013
9da0aa2
rename clone.py to a more descriptive clone_course.py
Aug 3, 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
27 changes: 21 additions & 6 deletions ...contentstore/management/commands/clone.py → ...store/management/commands/clone_course.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,14 @@
#
# To run from command line: rake cms:clone SOURCE_LOC=MITx/111/Foo1 DEST_LOC=MITx/135/Foo3
#
from request_cache.middleware import RequestCache
from django.core.cache import get_cache

#
# To run from command line: rake cms:delete_course LOC=MITx/111/Foo1
#

CACHE = get_cache('mongo_metadata_inheritance')

class Command(BaseCommand):
"""Clone a MongoDB-backed course to another location"""
Expand All @@ -21,19 +28,27 @@ class Command(BaseCommand):
def handle(self, *args, **options):
"Execute the command"
if len(args) != 2:
raise CommandError("clone requires two arguments: <source-location> <dest-location>")
raise CommandError("clone requires two arguments: <source-course_id> <dest-course_id>")

source_location_str = args[0]
dest_location_str = args[1]
source_course_id = args[0]
dest_course_id = args[1]

mstore = modulestore('direct')
cstore = contentstore()

print("Cloning course {0} to {1}".format(source_location_str, dest_location_str))
mstore.metadata_inheritance_cache_subsystem = CACHE
mstore.request_cache = RequestCache.get_request_cache()
org, course_num, run = dest_course_id.split("/")
mstore.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num))

source_location = CourseDescriptor.id_to_location(source_location_str)
dest_location = CourseDescriptor.id_to_location(dest_location_str)
print("Cloning course {0} to {1}".format(source_course_id, dest_course_id))

source_location = CourseDescriptor.id_to_location(source_course_id)
dest_location = CourseDescriptor.id_to_location(dest_course_id)

if clone_course(mstore, cstore, source_location, dest_location):
# be sure to recompute metadata inheritance after all those updates
mstore.refresh_cached_metadata_inheritance_tree(dest_location)
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't look at the file, just the diff; so, it may be obvious in context. (e.g., if there's no way to avoid the inheritance calc and doing it proactively squelches recomputes)

Since you're cloning and don't want to store inherited values in the clone but leave them "unset", why are you forcing metadata inheritance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just wanted to "warm the cache" after all these bulk updates happen. We do the same thing after import - suppress recomputation on the fly, because it isn't performant - and then do a single recompute at the end.


print("copying User permissions...")
_copy_course_group(source_location, dest_location)
15 changes: 11 additions & 4 deletions cms/djangoapps/contentstore/management/commands/delete_course.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,22 @@
from .prompt import query_yes_no

from auth.authz import _delete_course_group
from request_cache.middleware import RequestCache
from django.core.cache import get_cache

#
# To run from command line: rake cms:delete_course LOC=MITx/111/Foo1
#


CACHE = get_cache('mongo_metadata_inheritance')
class Command(BaseCommand):
help = '''Delete a MongoDB backed course'''

def handle(self, *args, **options):
if len(args) != 1 and len(args) != 2:
raise CommandError("delete_course requires one or more arguments: <location> |commit|")

loc_str = args[0]
course_id = args[0]

commit = False
if len(args) == 2:
Expand All @@ -34,9 +36,14 @@ def handle(self, *args, **options):
ms = modulestore('direct')
cs = contentstore()

if query_yes_no("Deleting course {0}. Confirm?".format(loc_str), default="no"):
ms.metadata_inheritance_cache_subsystem = CACHE
ms.request_cache = RequestCache.get_request_cache()
org, course_num, run = course_id.split("/")
ms.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num))

if query_yes_no("Deleting course {0}. Confirm?".format(course_id), default="no"):
if query_yes_no("Are you sure. This action cannot be undone!", default="no"):
loc = CourseDescriptor.id_to_location(loc_str)
loc = CourseDescriptor.id_to_location(course_id)
if delete_course(ms, cs, loc, commit):
print 'removing User permissions from course....'
# in the django layer, we need to remove all the user permissions groups associated with this course
Expand Down