fix: Don't look for a custom Python ZIP in Content Libs V2#37500
fix: Don't look for a custom Python ZIP in Content Libs V2#37500kdmccormick merged 1 commit intoopenedx:masterfrom
Conversation
Python-evaluated problems were failing to render because they were trying to invoke `course_id.make_asset_key` in order to obtain the asset key of the custom Python ZIP file. Learning Core assets work differently (no asset keys, etc.), and, furthermore, we haven't really thought though how and whether we want to support custom Python ZIPs in libraries. So, this fix punts on supporting Python ZIP files in libraries for now, but enables rendering of advanced CAPA problems which don't rely on a ZIP. This brings us to parity with Legacy Libraries, which didn't support assets at all and thus didn't support Python ZIPs either. Fixes: openedx#37447
kdmccormick
force-pushed
the
kdmccormick/python-eval-in-libs
branch
from
7286727 to
ee0db4a
Compare
| 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): |
There was a problem hiding this comment.
Just to double check my understanding: This works for both courses and v1 libs because v1 library keys are actually subclasses of CourseKey, right?
There was a problem hiding this comment.
@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:
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.
There was a problem hiding this comment.
(let me know if you think any of that warrants re-work or another code comment, happy to make a follow-up PR)
ormsbee
left a comment
ormsbee
left a comment
There was a problem hiding this comment.
One minor question to double check that I understand, but LGTM.
|
I haven't had time to test this, but a very cursory reading of the code makes me suspect that python_lib.zip will work for v1 libraries. @pdpinch: By any chance, do you know if MIT ever uses python_lib.zip in v1 libraries? |
|
Ah, python_lib.zip does work in v1 libraries if you manually create a Here's an example, using MIT OL's grading library. 
|
…7500) Python-evaluated problems were failing to render because they were trying to invoke `course_id.make_asset_key` in order to obtain the asset key of the custom Python ZIP file. Learning Core assets work differently (no asset keys, etc.), and, furthermore, we haven't really thought though how and whether we want to support custom Python ZIPs in libraries. So, this fix punts on supporting Python ZIP files in libraries for now, but enables rendering of advanced CAPA problems which don't rely on a ZIP. This brings us to parity with Legacy Libraries, which didn't support assets at all and thus didn't support Python ZIPs either. Fixes: openedx#37447
…7500) Python-evaluated problems were failing to render because they were trying to invoke `course_id.make_asset_key` in order to obtain the asset key of the custom Python ZIP file. Learning Core assets work differently (no asset keys, etc.), and, furthermore, we haven't really thought though how and whether we want to support custom Python ZIPs in libraries. So, this fix punts on supporting Python ZIP files in libraries for now, but enables rendering of advanced CAPA problems which don't rely on a ZIP. This brings us to parity with Legacy Libraries, which didn't support assets at all and thus didn't support Python ZIPs either. Fixes: openedx#37447
2 participants
Description
Python-evaluated problems were failing to render because they were trying to invoke
course_id.make_asset_keyin order to obtain the asset key of the custom Python ZIP file. Learning Core assets work differently (no asset keys, etc.), and, furthermore, we haven't really thought though how and whether we want to support custom Python ZIPs in libraries.So, this fix punts on supporting Python ZIP files in libraries for now, but enables rendering of advanced CAPA problems which don't rely on a ZIP. This brings us to parity with Legacy Libraries, which didn't support assets at all and thus didn't support Python ZIPs either.
Fixes:
loncapa/pythondoesn't render correctly on Library V2 #37447Supporting information
Python-eval'd problem in the lib sidebar preview
That same problem, linked into a course
Testing instructions
In the definition of
safe_exec, add the lineunsafely = True. This will allow you to render advanced python problems without codejail. (Obviously, this now allows LMS/CMS to run arbitrary course-authored Python on your computer, so remove this when you're done.)Then, go to the Demo Course, find the unit "Python-Evaluated Input". Copy "Simple Python-Evaluated Input Problem
", and paste it into a V2 library.
Confirm that the pasted library component can be previewed and edited. Confirm that you can submit a correct answer and an incorrect answer and that it evals correctly.
Publish the component and then use it in a course. Publish the course and view the component in the LMS. Confirm that you can submit a correct answer and an incorrect answer and that it evals correctly.
NOT TESTED: Codejail
I did not actually test this with Codejail running, as I couldn't get it set up on my machine in the time I had today. My thinking is that this PR, at worst, will break nothing, so we should be OK to merge and then test with Codejail on the sandbox.
Reviwers, if this is an issue, let me know and I can try to get local Codejail working next week.
Deadline
ASAP for Ulmo.