Repository navigation
Conversation
get_submission called warnings.filterwarnings('error') to turn jsonfield's
invalid-JSON RuntimeWarning back into an exception. warnings.filters is
shared by every thread, so under a threaded server any warning raised on
another request while one thread was inside get_submission became an
unhandled exception. The 'default' filter it added in `finally` also stayed
at the front of the list for good, overriding the host application's
ignore rules.
Detect corrupt answers in the field instead: Submission.answer now returns
invalid JSON as an InvalidJSONString marker, and get_submission raises
SubmissionInternalError on that, as before. deconstruct() keeps the parent
path, so no migration touches the submissions table.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the pull request, @Ardiea! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Description
get_submission()callswarnings.filterwarnings('error')before loading the submission andwarnings.filterwarnings('default')infinally. It does this to turn jsonfield's invalid-JSONRuntimeWarningback into an exception, which jsonfield raised itself before 3.1.0.warnings.filtersis a single list shared by the whole process. Under a threaded WSGI server (we run Granian with 8 blocking threads per worker), this causes two problems:get_submission, any warning raised on another thread is raised as an exception. We see this in production as unhandledDeprecationWarnings (500s) from code that has nothing to do with submissions. For example,datetime.utcnow()inSessionInactivityTimeout.process_requestfails on/event,/courseware-search/enabled/and xblock handler routes.filterwarnings('default')puts a catch-alldefaultfilter at the front of the list, and it stays there after the first call. That cancels edx-platform'signorerules inopenedx/core/lib/logsettings.py(for exampleSetting _field_data is deprecated). The('error', None, Warning, ...)entry also stays in the list, with only thatdefaultin front of it.warnings.catch_warnings()would not fix this. Before Python 3.14 it also swaps the process-wide filter list.Change
Submission.answernow usesAnswerJSONField, aJSONFieldsubclass. Itsfrom_db_valuereturns invalid JSON as anInvalidJSONString(aJSONStringsubclass) and still emits jsonfield'sRuntimeWarning. jsonfield wraps both valid JSON strings and invalid raw values inJSONString, so a separate marker type is needed to tell them apart.get_submission()checks forInvalidJSONStringand raisesSubmissionInternalErroras before. It no longer toucheswarnings.filters.AnswerJSONField.deconstruct()returnsjsonfield.fields.JSONFieldas the field path. No migration is generated (makemigrations --checkis clean), so nothing alters the submissions table. On SQLite anAlterFieldhere rebuilds the table.Answers are read and written the same way as before, and
get_submissionstill uses one query.Testing
test_load_non_json_answer(existing) still passes: a corruptraw_answerraisesSubmissionInternalErrorfromget_submissionandget_submission_and_student.test_get_submission_leaves_warning_filters_alone(string and dict answers) asserts thatwarnings.filtersis the same after the call. It fails on master.test_load_json_string_answer_that_looks_like_invalid_json: a valid JSON string answer'}'comes back as the answer and does not raise.make test_quality(pylint, isort, pycodestyle) is clean.🤖 Generated with Claude Code