From 35b2780658744c7c97903326fcdc7907ae1ee953 Mon Sep 17 00:00:00 2001
From: David Ormsbee <dave@edx.org>
Date: Wed, 7 Apr 2021 09:04:43 -0400
Subject: [PATCH] perf: avoid invoking codejail for problem response report

We don't need to execute instuctor code for the problem response
report because we're grabbing existing student state and scores.
Running codejail is expensive, slow, and risks report failures if
there is CPU intensive instructor code that is run at times when
the servers are already heavily loaded (and things start timing
out). This came up in TNL-8183 (and many others).
---
 common/lib/xmodule/xmodule/capa_module.py     |  5 ++++
 .../xmodule/xmodule/tests/test_capa_module.py | 23 ++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index 172128d907..5e7c6bceef 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -460,6 +460,11 @@ class ProblemBlock(
                     '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,
diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py
index a3351277b3..a176989c83 100644
--- a/common/lib/xmodule/xmodule/tests/test_capa_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py
@@ -3179,7 +3179,14 @@ class ProblemBlockReportGenerationTest(unittest.TestCase):
         scope_ids = Mock(block_type='problem')
         descriptor = ProblemBlock(get_test_system(), scope_ids=scope_ids)
         descriptor.runtime = Mock()
-        descriptor.data = '<problem/>'
+        # 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>
+        '''
         return descriptor
 
     def test_generate_report_data_not_implemented(self):
@@ -3210,3 +3217,17 @@ class ProblemBlockReportGenerationTest(unittest.TestCase):
         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