Performance enhancements for django-admin commands to delete and clone course #570
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
August 2, 2013 22:19
…speed up the whole thing as well as squelching warnings in the console output
|
|
||
| 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) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
Does this have existing unit tests which pass? If so, and if the clone does not set values which the source does not set, then 👍 |
|
@dmitchell re: tests. See test_clone_course() in test_contentstore.py |
chrisndodge
pushed a commit
that referenced
this pull request
Aug 6, 2013
…elete-course Performance enhancements for django-admin commands to delete and clone course
chrisndodge
deleted the
fix/cdodge/add-metadata-caching-to-delete-course
branch
chrisrossi
pushed a commit
to jazkarta/edx-platform
that referenced
this pull request
Mar 31, 2014
updated admin_dashboard to show count of unique students
bradenmacdonald
referenced
this pull request
in open-craft/edx-platform
Dec 4, 2015
Merge pull request #570 from edx-solutions/rc/2015-11-19
caesar2164
added a commit
to caesar2164/edx-platform
that referenced
this pull request
Jan 24, 2017
…block-sha Adding EdxAdapt xblock to platform
rediris
pushed a commit
to gymnasium/edx-platform
that referenced
this pull request
Feb 25, 2021
mariajgrimaldi
pushed a commit
to eduNEXT/edx-platform
that referenced
this pull request
Nov 2, 2021
Li/ednx/fd 4
Sujeet1379
pushed a commit
to chandrudev/edx-platform
that referenced
this pull request
Nov 17, 2022
These redirects are already in place for the courseware, and will redirect to the outline page, or the dashboard, or wherever, based on the type of access error (unenrolled, access expired, survey needed, etc). This commit stops the course home tabs from paying attention to the 401 error messages coming from the backend - course_access in the metadata API handles that now. This commit also changes our useModel hook to more gracefully handle not being able to find the model - it is a valid case we want to allow (still will cause problems if you actually try to use the data, but will at least provide an object you can inspect).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTES: This is done to vastly speed up deleting and cloning courses which are django-admins I run often regarding courseware manipulations. The problem was that there was no metadata cache being setup so there was alot of DB thrashing happening. Also we need to add the course_id to a special list of courses which are known to have a lot of grouped write transactions on it, so that final metadata computation is done at the end (for clone_course in this case).
@cahrens @dmitchell can you review?