Draft Grading Race Condition Fix

[ormsbee]

I'm experimenting on top of @marslanabdulrauf's #37825 to see if I can get the test failures ironed out without having to change the outward behavior of modulestore.

(And hopefully without breaking other stuff.)

[ormsbee]

So the errors happening on this one are due to the fact that it's legal (though unusual) to query the modulestore for a CourseKey where nothing is set but the version_guid (since that points to a unique structure document):

=========================== short test summary info ============================
FAILED xmodule/modulestore/tests/test_split_modulestore.py::SplitModuleCourseTests::test_get_course - opaque_keys.InvalidKeyError: <class 'opaque_keys.edx.locator.CourseLocator'>: Either version_guid or org, course, and run should be set
FAILED xmodule/modulestore/tests/test_split_modulestore.py::SplitModuleItemTests::test_get_item - opaque_keys.InvalidKeyError: <class 'opaque_keys.edx.locator.CourseLocator'>: Either version_guid or org, course, and run should be set
FAILED xmodule/modulestore/tests/test_split_modulestore.py::TestItemCrud::test_create_minimal_item - opaque_keys.InvalidKeyError: <class 'opaque_keys.edx.locator.CourseLocator'>: Either version_guid or org, course, and run should be set
FAILED xmodule/modulestore/tests/test_split_modulestore.py::TestItemCrud::test_update_metadata - opaque_keys.InvalidKeyError: <class 'opaque_keys.edx.locator.CourseLocator'>: Either version_guid or org, course, and run should be set
==== 4 failed, 1524 passed, 180 skipped, 1840 warnings in 399.27s (0:06:39) ====

I don't know why on earth we would be pulling this value from the PartitionService, but I'm sure the answer will be fun.

[ormsbee]

@bradenmacdonald, @kdmccormick, @feanil: Please take a quick look at this fix for the release blocker. I was planning to push this to #37825 because that's the PR that should eventually merge (it has a lot of useful discussion). But I don't seem to have permission to push to that branch, so I'm putting the code up here for now.

[bradenmacdonald]

Oh dear. I wish we didn't have such flexible CourseKeys that can hold such a variety of different fields. Thanks for the nice clear comments!

From a quick look, this seems sane and reasonable, but I am unclear how the partition services changes fix the bug nor how that was being used.

[ormsbee]

@bradenmacdonald: The PartitionService change does nothing for the original bug (that's solved by doing a copy() on the services dict). The PartitionService change was necessary because tests were breaking because now that there truly were multiple instances of the PartitionService floating around, it mattered that they disagreed on the version_guid of the course key. (Previously, they all just pointed to the same instance, and that instance was replaced every time a runtime was instantiated).

What this would have meant in terms of real-world impact is that if you made a publish and then tried to do an access check on another piece of content that had its runtime created before your publish, that access check would blow up. Which isn't a common case, but could happen.