Skip to content

fix: Don't look for a custom Python ZIP in Content Libs V2 #37500

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

Merged
kdmccormick merged 1 commit into from
Oct 17, 2025
Merged

fix: Don't look for a custom Python ZIP in Content Libs V2 #37500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 18 additions & 1 deletion common/djangoapps/util/tests/test_sandboxing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from django.test import TestCase
from django.test.utils import override_settings
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import CourseLocator, LibraryLocator
from opaque_keys.edx.locator import CourseLocator, LibraryLocator, LibraryLocatorV2

from xmodule.contentstore.django import contentstore
from xmodule.modulestore.tests.django_utils import upload_file_to_course
Expand Down Expand Up @@ -114,3 +114,20 @@ def test_get_python_lib_zip(self):

def test_no_python_lib_zip(self):
assert self.sandbox_service.get_python_lib_zip() is None


class SandboxServiceForLibrariesV2Test(TestCase):
"""
Test SandboxService methods for V2 Content Libraries.

(Lacks tests for anything other than python_lib_zip)
"""

@classmethod
def setUpClass(cls):
super().setUpClass()
library_key = LibraryLocatorV2('test', 'sandbox_test')
cls.sandbox_service = SandboxService(course_id=library_key, contentstore=contentstore)

def test_no_python_lib_zip(self):
assert self.sandbox_service.get_python_lib_zip() is None
9 changes: 7 additions & 2 deletions xmodule/util/sandboxing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import re

from django.conf import settings
from opaque_keys.edx.keys import CourseKey, LearningContextKey

DEFAULT_PYTHON_LIB_FILENAME = 'python_lib.zip'

Expand Down Expand Up @@ -39,10 +40,14 @@ def can_execute_unsafe_code(course_id):
return False


def get_python_lib_zip(contentstore, course_id):
def get_python_lib_zip(contentstore, context_key: LearningContextKey):
"""Return the bytes of the course code library file, if it exists."""
if not isinstance(context_key, CourseKey):
Copy link
Contributor

@ormsbee ormsbee Oct 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Just to double check my understanding: This works for both courses and v1 libs because v1 library keys are actually subclasses of CourseKey, right?

Copy link
Member Author

@kdmccormick kdmccormick Oct 17, 2025

Choose a reason for hiding this comment

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

@ormsbee kinda. V1 LibraryKeys are subclasses of CourseKeys, so they will skip this conditional, and we will call contentestore().find(...), and then when we don't find the ZIP, the rendering will happily chug along anyway (because throw_on_not_found=False).

Here's it in action in V1 libraries:

Screenshot 2025-10-17 at 5 27 11 PM

I don't actually know what happens if an author puts a python zip file in their v1 library OLX and uploads it. I know that v1 libraries "don't support assets" but I'm not clear if that's a UI-level or backend-level thing. In any case, I don't think this PR should affect anything with V1 libraries.

Copy link
Member Author

@kdmccormick kdmccormick Oct 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

(let me know if you think any of that warrants re-work or another code comment, happy to make a follow-up PR)

# Although Content Libraries V2 does support python-evaluated capa problems,
# it doesn't yet support supplementary python zip files.
return None
python_lib_filename = course_code_library_asset_name()
asset_key = course_id.make_asset_key("asset", python_lib_filename)
asset_key = context_key.make_asset_key("asset", python_lib_filename)
zip_lib = contentstore().find(asset_key, throw_on_not_found=False)
if zip_lib is not None:
return zip_lib.data
Expand Down
Loading