Skip to content

Revert "perf: avoid invoking codejail for problem response report" (#27310) #28206

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
nedbat merged 1 commit into from
Jul 16, 2021
Merged

Revert "perf: avoid invoking codejail for problem response report" (#27310) #28206

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
5 changes: 0 additions & 5 deletions common/lib/xmodule/xmodule/capa_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,11 +460,6 @@ def generate_report_data(self, user_state_iterator, limit_responses=None):
'seed': user_state.state.get('seed'),
},
seed=user_state.state.get('seed'),
# The main point of minimal_init=True here is to avoid invoking
# codejail, which makes the reports take much longer to run,
# and can also cause failures for CPU intensive instructor code
# in problems.
minimal_init=True,
# extract_tree=False allows us to work without a fully initialized CapaModule
# We'll still be able to find particular data in the XML when we need it
extract_tree=False,
Expand Down
23 changes: 1 addition & 22 deletions common/lib/xmodule/xmodule/tests/test_capa_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3179,14 +3179,7 @@ def _get_descriptor(self): # lint-amnesty, pylint: disable=missing-function-doc
scope_ids = Mock(block_type='problem')
descriptor = ProblemBlock(get_test_system(), scope_ids=scope_ids)
descriptor.runtime = Mock()
# Put a script tag so that codejail is normally invoked, to test that we
# suppress that invocation when generating this report.
descriptor.data = '''<problem>
<script type="loncapa/python">
x1 = random.randint(0, 100)
</script>
</problem>
'''
descriptor.data = '<problem/>'
return descriptor

def test_generate_report_data_not_implemented(self):
Expand Down Expand Up @@ -3217,17 +3210,3 @@ def test_generate_report_data_skip_dynamath(self):
iterator = iter([self._user_state(suffix='_dynamath')])
report_data = list(descriptor.generate_report_data(iterator))
assert 0 == len(report_data)

def test_safe_exec_not_called(self):
"""
Make sure we're not calling instructor code when doing this report.

This relies on us passing minimal_init=True when making the
LoncapaProblem. Without that, this whole suite will break because the
data in the descriptor (self._get_descriptor()) will force capa to do
initializations that are mocked out at the moment.
"""
with patch('capa.safe_exec.safe_exec') as mock_safe_exec:
descriptor = self._get_descriptor()
list(descriptor.generate_report_data(self._mock_user_state_generator(), 2))
assert not mock_safe_exec.called