This commit implements a comprehensive solution for test score integration in the
enhancement system along with improvements to the score rendering mechanism. Key
changes include:
- Add event handler for rendering blocks with edx-submissions scores
- Implement event-based mechanism to render XBlocks with scoring data
- Create signal handlers in handlers.py to process external grader scores
- Develop specialized XBlock loader for rendering without HTTP requests
- Add queue_key propagation across the submission pipeline
- Register submission URLs in LMS routing configuration
- Add complete docstrings to score render module for better code maintainability
- Add ADR for XBlock rendering with external grader integration
- Add openedx-events fork branch as a dependency in testing.in
- Upgrade edx submission dependency
These changes support the migration from traditional XQueue callback HTTP requests
to a more robust event-based architecture, improving performance and reliability
when processing submission scores. The included ADR documents the architectural
decision and implementation approach for this significant improvement to the
external grading workflow.
* Consolidates and renames the runtime used as a base for all the others:
* Before: `xmodule.x_module:DescriptorSystem` and
`xmodule.mako_block:MakoDescriptorSystem`.
* After: `xmodule.x_module:ModuleStoreRuntime`.
* Co-locates and renames the runtimes for importing course OLX:
* Before: `xmodule.x_module:XMLParsingSystem` and
`xmodule.modulestore.xml:ImportSystem`.
* After: `xmodule.modulestore.xml:XMLParsingModuleStoreRuntime` and
`xmodule.modulestore.xml:XMLImportingModuleStoreRuntime`.
* Note: I would have liked to consolidate these, but it would have
involved nontrivial test refactoring.
* Renames the stub Old Mongo runtime:
* Before: `xmodule.modulestore.mongo.base:CachingDescriptorSystem`.
* After: `xmodule.modulestore.mongo.base:OldModuleStoreRuntime`.
* Renames the Split Mongo runtime, the which is what runs courses in LMS and CMS:
* Before: `xmodule.modulestore.split_mongo.caching_descriptor_system:CachingDescriptorSystem`.
* After: `xmodule.modulestore.split_mongo.runtime:SplitModuleStoreRuntime`.
* Renames some of the dummy runtimes used only in unit tests.
This brings an important security improvement -- codejail won't default to
running in unsafe mode, which can happen if certain configuration errors
are present.
Properly configured installations shouldn't be affected. We just need to
adjust some unit tests to opt into unsafe mode.
Changes:
- Update `edx-codejail` dependency to [version 4.0.0](https://github.com/openedx/codejail/blob/master/CHANGELOG.rst#400---2025-06-13)
- Define a `use_unsafe_codejail` decorator that allows running a unit test (or entire TestCase class) in unsafe mode
- Use that decorator as needed, based on which tests started failing
Some of the calls to `safe_exec` were missing the `limit_overrides_context`
parameter. This normally conveys the course key to codejail so that we can
give some courses different resource limits, but it's also valuable for
diagnosing codejail issues in logs and other telemetry.
I wasn't able to test all of these paths manually, but the utility function
`get_course_id_from_capa_block` will swallow errors, so the situation
should be no worse if the `LoncapaResponse.capa_block` field has something
unexpected.
For darklaunch comparisons where the two sides have different Python
versions, we'll want a more comprehensive list of normalizers.
- Expand the default list to include patterns discovered during a Python
3.8 vs. 3.12 comparison.
- Append the setting value by default, rather than replacing (but still
allow replacing).
- Use default normalizers if custom ones can't be loaded.
- Add log message when loading normalizers fails.
- Validate the replacement pattern, not just the search pattern.
This is intended to make logs more or less a standalone source for
analyzing mismatches.
- Only log mismatches or exceptions
- Merge local and remote log messages into one so they can be correlated
more easily
- Different log messages for mismatch vs. unexpected exceptions
- Include course ID (as limit overrides context) in log message
- Fix bug where we were overwriting `remote_emsg` with None, and add test
that would have caught it.
- Suppress differences due solely to the codejail sandbox directory name
differing (in stack traces), and add test for this. Configurable because
we'll need to add an additional search/replace pair for the sandbox venv
paths.
- Add a variety of custom attributes, replacing existing ones. The attrs
now have a prefixed naming scheme to simplify searching.
- Add slug to log output so we can more readily correlate traces and logs,
as well as logs across services.
- Fix typo in error message.
- Fix existing import sort order lint.
- Separate test for misconfiguration
- Add helper method for generic dark launch testing
- Test two darklaunch scenarios: Globals interference, and error that would
previously have caused the remote side not to run
- Rename mocks to have our usual `mock_` prefix
- Catch all exceptions, not just Exception, to better prevent errors from
interfering with mainline responses.
- Introduce a separate try block around the monitoring code so that bugs
there don't cause issues.
- Print exception information as well for both sides (but only if not a
SafeExecException, which is redundant with emsg).
Some formatting changes to log messages as well.
Example outputs:
For `1/0`:
```
2025-04-14 17:26:34,239 INFO 10232 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:240 - Remote execution in darklaunch mode produces globals={'expect': None, 'ans': '1/0'}, emsg=None, exception=None
2025-04-14 17:26:34,239 INFO 10232 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:245 - Local execution in darklaunch mode produces globals={'expect': None, 'ans': '1/0'}, emsg='ZeroDivisionError: division by zero', exception=None
```
For `raise BaseException("hi")`:
```
2025-04-14 17:26:13,359 INFO 10232 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:240 - Remote execution in darklaunch mode produces globals={'expect': None, 'ans': 'raise BaseException("hi")'}, emsg=None, exception=None
2025-04-14 17:26:13,359 INFO 10232 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:245 - Local execution in darklaunch mode produces globals={'expect': None, 'ans': 'raise BaseException("hi")'}, emsg='hi', exception=BaseException('hi')
```
With codejail-service down, and `out = 1 + 2`:
```
2025-04-14 17:30:28,597 INFO 12484 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:241 - Remote execution in darklaunch mode produces globals={'expect': None, 'ans': 'out = 1 + 2', 'out': 3, 'cfn_return': {'input_list': [{'ok': True, 'msg': 'Output:\n3', 'grade_decimal': 1}]}}, emsg=None, exception=CodejailServiceUnavailable('Codejail API Service is unavailable. Please try again in a few minutes.')
2025-04-14 17:30:28,597 INFO 12484 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:246 - Local execution in darklaunch mode produces globals={'expect': None, 'ans': 'out = 1 + 2', 'out': 3, 'cfn_return': {'input_list': [{'ok': True, 'msg': 'Output:\n3', 'grade_decimal': 1}]}}, emsg=None, exception=None
```
During dark launch of remote codejail, we want to ensure we always run both
local and remote execution -- otherwise we're missing data for the remote
side in an important situation.
This will help answer the question of whether the unexpected exception
happens on both sides, even though it may not look exactly the same due to
differences in how unexpected errors are handled.
An example input that provokes this in unsafe execution mode is
`raise BaseException("hi")`; in safe execution mode, printing to
`sys.__stdout__` should also produce an appropriate error.
fix: Restructuring to send course_id and score to edx submission
fix: Refactoring of sending information to sent_to_submission
fix: Elimination of unnecessary functions
fix: Added usage comment to ProblemBlock in XqueueInterface constructor
fix: update doc ADR
fix: setting for Quality Others test (ubuntu-24.04, 3.11, 20)
fix: Deprecation for django-trans-escape-filter-parse-error
test: Add @pytest.mark.django_db decorator to test functions
test: Fix for pylint being disabled
fix: updated changes for pylint disable
fix: update error from docs ADR-0005
update: xmodule/docs/decisions/0005-send-data-to-edx-submission.rst
Co-authored-by: Sarina Canelake <sarina@axim.org>
update: xmodule/docs/decisions/0005-send-data-to-edx-submission.rst
Co-authored-by: Sarina Canelake <sarina@axim.org>
fix: Adjusted correction
fix: update date for docs ADR
Revert "fix: update date for docs ADR"
This reverts commit 0b4229c51c4937f95cb407872645dd448df45418.
fix: replace call created_submission to create_external_grader_detail
fix: update test xqueue_submission
fix: add docstring in test_xqueue_submission
fix: update date doc ADR
fix: update version edx-submission 3.8.6
fix: add @pytest.mark.xfail
fix: add 20 chances in test_capa_block:
fix: increase retry attempts for seed generation in ProblemBlockTest
fix: change version to edx-submission lib
fix: new version edx-submission in testings
fix: replace parameter file to files
fix: update variable grader_file_name and points_possible
fix: Adjustment in the is_flag_active function to always take the last record edited in the waffle
fix: wrap large line of code
fix: update function is_flag_active
fix: code style adjustment
fix: changes for 60 retry
feat: use CourseWaffleFlag to determine xqueue callback path
fix: Code style adjustment
fix: remove deprecated xqueue callback route and simplify callback type logic
fix: Deleting a comment in the ADR document
fix: add log in self.block is None
fix: Code style adjustment in log
Example output from running `import matplotlib; 1/0`, before and after the change:
```diff
--- tmp/before 2025-03-28 03:34:06.633689552 +0000
+++ tmp/after 2025-03-28 03:34:37.268688891 +0000
@@ -1,6 +1,5 @@
-Matplotlib created a temporary cache directory at /tmp/codejail-hveq16ah/tmp/matplotlib-tv0c_vzt because the default path (/home/sandbox/.config/matplotlib) is not a writable directory; it is highly recommended to set the MPLCONFIGDIR environment variable to a writable directory, in particular to speed up the import of Matplotlib and to better support multiprocessing.
Traceback (most recent call last):
File "jailed_code", line 19, in <module>
exec(code, g_dict)
File "<string>", line 1, in <module>
ZeroDivisionError: division by zero
```
This makes it easier to run matplotlib in codejail, and should prevent a
number of other issues in the future with other packages that need to
create tempfiles.
No change is required for existing codejail installations, but after this
change operators may be able to tighten their apparmor configuration to
prevent write access to global temp or cache dirs.
Manual testing instructions: Create a codejail problem that runs
`import matplotlib` and confirm that it runs without error. (Unit tests
aren't feasible here because this requires a fully configured codejail in
order for the tmp subdirectory to exist.)
Also: Add comment for `OPENBLAS_NUM_THREADS` and numpy support.
As of Python 3.3, the 3rd-party `mock` package has been subsumed into the
standard `unittest.mock` package. Refactoring tests to use the latter will
allow us to drop `mock` as a dependency, which is currently coming in
transitively through requirements/edx/paver.in.
We don't actually drop the `mock` dependency in this PR. That will happen
naturally in:
* https://github.com/openedx/edx-platform/pull/34830
A new field in the Problem settings for choosing a Grading Method. Currently, the only Grading Method is the Last Score. From now on, when turning the feature flag on, the new grading methods available for configuration in Studio are:
- Last Score (Default): The last score made is taken for grading.
- First Score: The first score made is taken for grading.
- Highest Score: The highest score made is taken for grading.
- Average Score: The average of all scores made is taken for grading.
Currently, ./xmodule/ unit tests are only run with LMS settings. However,
./common/ and ./xmodule/ are run twice: once with LMS settings and once with
CMS settings.
Just like ./common/ and ./openedx/, the unit tests in ./xmodule/ validate
behavior in both LMS and CMS. So, order to fully test ./xmodule/, we should to
run its tests with CMS settings too.
This will enable us to better validate certain LibraryContentBlocks behaviors
being touched by https://github.com/openedx/edx-platform/pull/33263 which can't
be expressed under LMS settings.
Also in this commit:
* refactor: rename the shards to be clear whether they're running under LMS or CMS
* docs: correct comments regarding conditions under which codejail's
test_cant_do_something_forbidden is skipped.
* test: update a unit test which was using the now-deleted library_sourced block to use
library_content block instead.
* get rid of six.text_type(s)
* get rid of six.b()
* get rid of six.string_types
* get rid of six.PY2/six.PY3
* get rid of six.iteritems() and six.viewvalues()
The XQueueService is used only by the ProblemBlock. Therefore, we are moving
it out of the runtime, and into the ProblemBlock, where it's initialized only
when it's going to be used.
