Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: move XQueueService from the runtime to ProblemBlock

Browse Source 
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.
This commit is contained in:
Piotr Surowiec
2023-06-21 20:08:31 +02:00
committed by GitHub
parent 9e0b59fe87
commit 0137f21627
7 changed files with 74 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
lms/djangoapps/courseware/block_render.py
View File

@@ -53,7 +53,6 @@ from xmodule.util.sandboxing import SandboxService
from xmodule.services import EventPublishingService, RebindUserService, SettingsService, TeamsConfigurationService
from common.djangoapps.static_replace.services import ReplaceURLService
from common.djangoapps.static_replace.wrapper import replace_urls_wrapper
from xmodule.capa.xqueue_interface import XQueueService # lint-amnesty, pylint: disable=wrong-import-order
from lms.djangoapps.courseware.access import get_user_role, has_access
from lms.djangoapps.courseware.entrance_exams import user_can_skip_entrance_exam, user_has_passed_entrance_exam
from lms.djangoapps.courseware.masquerade import (
@@ -439,44 +438,12 @@ def prepare_runtime_for_user(
KvsFieldData: student_data bound to, primarily, the user and block
"""
def make_xqueue_callback(dispatch='score_update'):
"""
Returns fully qualified callback URL for external queueing system
"""
relative_xqueue_callback_url = reverse(
'xqueue_callback',
kwargs=dict(
course_id=str(course_id),
userid=str(user.id),
mod_id=str(block.location),
dispatch=dispatch
),
)
xqueue_callback_url_prefix = settings.XQUEUE_INTERFACE.get('callback_url', settings.LMS_ROOT_URL)
return xqueue_callback_url_prefix + relative_xqueue_callback_url
# Default queuename is course-specific and is derived from the course that
# contains the current block.
# TODO: Queuename should be derived from 'course_settings.json' of each course
xqueue_default_queuename = block.location.org + '-' + block.location.course
xqueue_service = XQueueService(
construct_callback=make_xqueue_callback,
default_queuename=xqueue_default_queuename,
url=settings.XQUEUE_INTERFACE['url'],
django_auth=settings.XQUEUE_INTERFACE['django_auth'],
basic_auth=settings.XQUEUE_INTERFACE.get('basic_auth'),
waittime=settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS,
)
def inner_get_block(block):
"""
Delegate to get_block_for_descriptor_internal() with all values except `block` set.
Because it does an access check, it may return None.
"""
# TODO: fix this so that make_xqueue_callback uses the block passed into
# inner_get_block, not the parent's callback. Add it as an argument....
return get_block_for_descriptor_internal(
user=user,
block=block,
@@ -579,7 +546,6 @@ def prepare_runtime_for_user(
'content_type_gating': ContentTypeGatingService(),
'cache': CacheService(cache),
'sandbox': SandboxService(contentstore=contentstore, course_id=course_id),
'xqueue': xqueue_service,
'replace_urls': replace_url_service,
# Rebind module service to deal with noauth modules getting attached to users.
'rebind_user': RebindUserService(

44
lms/djangoapps/courseware/tests/test_block_render.py
View File

@@ -41,7 +41,6 @@ from xblock.runtime import DictKeyValueStore, KvsFieldData # lint-amnesty, pyli
from xblock.test.tools import TestRuntime # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.capa.tests.response_xml_factory import OptionResponseXMLFactory # lint-amnesty, pylint: disable=reimported
from xmodule.capa.xqueue_interface import XQueueInterface
from xmodule.capa_block import ProblemBlock
from xmodule.contentstore.django import contentstore
from xmodule.html_block import AboutBlock, CourseInfoBlock, HtmlBlock, StaticTabBlock
@@ -121,7 +120,6 @@ TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT
@XBlock.needs('content_type_gating')
@XBlock.needs('cache')
@XBlock.needs('sandbox')
@XBlock.needs('xqueue')
@XBlock.needs('replace_urls')
@XBlock.needs('rebind_user')
@XBlock.needs('completion')
@@ -2333,7 +2331,6 @@ class LMSXBlockServiceBindingTest(LMSXBlockServiceMixin):
'content_type_gating',
'cache',
'sandbox',
'xqueue',
'replace_urls',
'rebind_user',
'completion',
@@ -2865,47 +2862,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
rendered = self.block.runtime.render_template('templates/edxmako.html', {'element_id': 'hi'}) # pylint: disable=not-callable
assert rendered == '<div id="hi" ns="main">Testing the MakoService</div>\n'
def test_xqueue(self):
xqueue = self.block.runtime.xqueue
assert isinstance(xqueue['interface'], XQueueInterface)
assert xqueue['interface'].url == 'http://sandbox-xqueue.edx.org'
assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest'
assert xqueue['waittime'] == 5
callback_url = f'http://localhost:8000/courses/{self.course.id}/xqueue/232/{self.block.location}'
assert xqueue['construct_callback']() == f'{callback_url}/score_update'
assert xqueue['construct_callback']('mock_dispatch') == f'{callback_url}/mock_dispatch'
@override_settings(
XQUEUE_INTERFACE={
'callback_url': 'http://alt.url',
'url': 'http://xqueue.url',
'django_auth': {
'username': 'user',
'password': 'password',
},
'basic_auth': ('basic', 'auth'),
},
XQUEUE_WAITTIME_BETWEEN_REQUESTS=15,
)
def test_xqueue_settings(self):
_ = render.prepare_runtime_for_user(
self.user,
self.student_data,
self.block,
self.course.id,
self.track_function,
self.request_token,
course=self.course,
)
xqueue = self.block.runtime.xqueue
assert isinstance(xqueue['interface'], XQueueInterface)
assert xqueue['interface'].url == 'http://xqueue.url'
assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest'
assert xqueue['waittime'] == 15
callback_url = f'http://alt.url/courses/{self.course.id}/xqueue/232/{self.block.location}'
assert xqueue['construct_callback']() == f'{callback_url}/score_update'
assert xqueue['construct_callback']('mock_dispatch') == f'{callback_url}/mock_dispatch'
@override_settings(COURSES_WITH_UNSAFE_CODE=[r'course-v1:edX\+LmsModuleShimTest\+2021_Fall'])
def test_can_execute_unsafe_code_when_allowed(self):
assert self.block.runtime.can_execute_unsafe_code()

53
xmodule/capa/tests/test_xqueue_interface.py
View File

@@ -1,41 +1,52 @@
# -*- coding: utf-8 -*-
"""
Tests the xqueue service interface.
"""
"""Test the XQueue service and interface."""
from unittest import TestCase
from unittest.mock import Mock
from django.conf import settings
from django.test.utils import override_settings
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
from xblock.fields import ScopeIds
from xmodule.capa.xqueue_interface import XQueueInterface, XQueueService
class XQueueServiceTest(TestCase):
"""
Tests the XQueue service methods.
"""
@staticmethod
def construct_callback(*args, **kwargs):
return 'https://lms.url/callback'
"""Test the XQueue service methods."""
def setUp(self):
super().setUp()
self.service = XQueueService(
url=settings.XQUEUE_INTERFACE['url'],
django_auth=settings.XQUEUE_INTERFACE['django_auth'],
basic_auth=settings.XQUEUE_INTERFACE['basic_auth'],
construct_callback=self.construct_callback,
default_queuename='my-very-own-queue',
waittime=settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS,
)
location = BlockUsageLocator(CourseLocator("test_org", "test_course", "test_run"), "problem", "ExampleProblem")
self.block = Mock(scope_ids=ScopeIds('user1', 'mock_problem', location, location))
self.service = XQueueService(self.block)
def test_interface(self):
"""Test that the `XQUEUE_INTERFACE` settings are passed from the service to the interface."""
assert isinstance(self.service.interface, XQueueInterface)
assert self.service.interface.url == 'http://sandbox-xqueue.edx.org'
assert self.service.interface.auth['username'] == 'lms'
assert self.service.interface.auth['password'] == '***REMOVED***'
assert self.service.interface.session.auth.username == 'anant'
assert self.service.interface.session.auth.password == 'agarwal'
def test_construct_callback(self):
assert self.service.construct_callback() == 'https://lms.url/callback'
"""Test that the XQueue callback is initialized correctly, and can be altered through the settings."""
usage_id = self.block.scope_ids.usage_id
callback_url = f'courses/{usage_id.context_key}/xqueue/user1/{usage_id}'
assert self.service.construct_callback() == f'{settings.LMS_ROOT_URL}/{callback_url}/score_update'
assert self.service.construct_callback('alt_dispatch') == f'{settings.LMS_ROOT_URL}/{callback_url}/alt_dispatch'
custom_callback_url = 'http://alt.url'
with override_settings(XQUEUE_INTERFACE={**settings.XQUEUE_INTERFACE, 'callback_url': custom_callback_url}):
assert self.service.construct_callback() == f'{custom_callback_url}/{callback_url}/score_update'
def test_default_queuename(self):
assert self.service.default_queuename == 'my-very-own-queue'
"""Check the format of the default queue name."""
assert self.service.default_queuename == 'test_org-test_course'
def test_waittime(self):
"""Check that the time between requests is retrieved correctly from the settings."""
assert self.service.waittime == 5
with override_settings(XQUEUE_WAITTIME_BETWEEN_REQUESTS=15):
assert self.service.waittime == 15

62
xmodule/capa/xqueue_interface.py
View File

@@ -1,13 +1,19 @@
"""
LMS Interface to external queueing system (xqueue)
"""
from typing import Dict, Optional, TYPE_CHECKING
import hashlib
import json
import logging
import requests
import six
from django.conf import settings
from django.urls import reverse
from requests.auth import HTTPBasicAuth
if TYPE_CHECKING:
from xmodule.capa_block import ProblemBlock
log = logging.getLogger(__name__)
dateformat = '%Y%m%d%H%M%S'
@@ -25,7 +31,7 @@ def make_hashkey(seed):
Generate a string key by hashing
"""
h = hashlib.md5()
h.update(six.b(str(seed)))
h.update(str(seed).encode('latin-1'))
return h.hexdigest()
@@ -65,13 +71,13 @@ def parse_xreply(xreply):
return (return_code, content)
class XQueueInterface(object):
class XQueueInterface:
"""
Interface to the external grading system
"""
def __init__(self, url, django_auth, requests_auth=None):
self.url = six.text_type(url)
def __init__(self, url: str, django_auth: Dict[str, str], requests_auth: Optional[HTTPBasicAuth] = None):
self.url = url
self.auth = django_auth
self.session = requests.Session()
self.session.auth = requests_auth
@@ -155,21 +161,17 @@ class XQueueService:
XBlock service providing an interface to the XQueue service.
Args:
construct_callback(callable): function which constructs a fully-qualified callback URL to make xqueue requests.
default_queuename(string): course-specific queue name.
waittime(int): number of seconds to wait between xqueue requests
url(string): base URL for the XQueue service.
django_auth(dict): username and password for the XQueue service.
basic_auth(array or None): basic authentication credentials, if needed.
block: The `ProblemBlock` instance.
"""
def __init__(self, construct_callback, default_queuename, waittime, url, django_auth, basic_auth=None):
requests_auth = requests.auth.HTTPBasicAuth(*basic_auth) if basic_auth else None
self._interface = XQueueInterface(url, django_auth, requests_auth)
def __init__(self, block: 'ProblemBlock'):
basic_auth = settings.XQUEUE_INTERFACE.get('basic_auth')
requests_auth = HTTPBasicAuth(*basic_auth) if basic_auth else None
self._interface = XQueueInterface(
settings.XQUEUE_INTERFACE['url'], settings.XQUEUE_INTERFACE['django_auth'], requests_auth
)
self._construct_callback = construct_callback
self._default_queuename = default_queuename.replace(' ', '_')
self._waittime = waittime
self._block = block
@property
def interface(self):
@@ -178,23 +180,33 @@ class XQueueService:
"""
return self._interface
@property
def construct_callback(self):
def construct_callback(self, dispatch: str = 'score_update') -> str:
"""
Returns the function to construct the XQueue callback.
Return a fully qualified callback URL for external queueing system.
"""
return self._construct_callback
relative_xqueue_callback_url = reverse(
'xqueue_callback',
kwargs=dict(
course_id=str(self._block.scope_ids.usage_id.context_key),
userid=str(self._block.scope_ids.user_id),
mod_id=str(self._block.scope_ids.usage_id),
dispatch=dispatch,
),
)
xqueue_callback_url_prefix = settings.XQUEUE_INTERFACE.get('callback_url', settings.LMS_ROOT_URL)
return xqueue_callback_url_prefix + relative_xqueue_callback_url
@property
def default_queuename(self):
def default_queuename(self) -> str:
"""
Returns the default queue name for the current course.
"""
return self._default_queuename
course_id = self._block.scope_ids.usage_id.context_key
return f'{course_id.org}-{course_id.course}'.replace(' ', '_')
@property
def waittime(self):
def waittime(self) -> int:
"""
Returns the number of seconds to wait in between calls to XQueue.
"""
return self._waittime
return settings.XQUEUE_WAITTIME_BETWEEN_REQUESTS

9
xmodule/capa_block.py
View File

@@ -52,6 +52,7 @@ from common.djangoapps.xblock_django.constants import (
ATTR_KEY_USER_ID,
)
from openedx.core.djangolib.markup import HTML, Text
from .capa.xqueue_interface import XQueueService
from .fields import Date, ScoreField, Timedelta
from .progress import Progress
@@ -123,8 +124,6 @@ class Randomization(String):
@XBlock.needs('cache')
@XBlock.needs('sandbox')
@XBlock.needs('replace_urls')
# Studio doesn't provide XQueueService, but the LMS does.
@XBlock.wants('xqueue')
@XBlock.wants('call_to_action')
class ProblemBlock(
ScorableXBlockMixin,
@@ -814,6 +813,8 @@ class ProblemBlock(
sandbox_service = self.runtime.service(self, 'sandbox')
cache_service = self.runtime.service(self, 'cache')
is_studio = getattr(self.runtime, 'is_author_mode', False)
capa_system = LoncapaSystem(
ajax_url=self.ajax_url,
anonymous_student_id=anonymous_student_id,
@@ -825,7 +826,7 @@ class ProblemBlock(
render_template=self.runtime.service(self, 'mako').render_template,
resources_fs=self.runtime.resources_fs,
seed=seed, # Why do we do this if we have self.seed?
xqueue=self.runtime.service(self, 'xqueue'),
xqueue=None if is_studio else XQueueService(self),
matlab_api_key=self.matlab_api_key
)
@@ -1736,7 +1737,7 @@ class ProblemBlock(
if self.lcp.is_queued():
prev_submit_time = self.lcp.get_recentmost_queuetime()
xqueue_service = self.runtime.service(self, 'xqueue')
xqueue_service = self.lcp.capa_system.xqueue
waittime_between_requests = xqueue_service.waittime if xqueue_service else 0
if (current_time - prev_submit_time).total_seconds() < waittime_between_requests:
msg = _("You must wait at least {wait} seconds between submissions.").format(

16
xmodule/tests/__init__.py
View File

@@ -161,14 +161,6 @@ def get_test_system(
services = {
'user': user_service,
'mako': mako_service,
'xqueue': XQueueService(
url='http://xqueue.url',
django_auth={},
basic_auth=[],
default_queuename='testqueue',
waittime=10,
construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"),
),
'replace_urls': replace_url_service,
'cache': CacheService(DoNothingCache()),
'field-data': DictFieldData({}),
@@ -224,14 +216,6 @@ def prepare_block_runtime(
services = {
'user': user_service,
'mako': mako_service,
'xqueue': XQueueService(
url='http://xqueue.url',
django_auth={},
basic_auth=[],
default_queuename='testqueue',
waittime=10,
construct_callback=Mock(name='get_test_system.xqueue.construct_callback', side_effect="/"),
),
'replace_urls': replace_url_service,
'cache': CacheService(DoNothingCache()),
'field-data': DictFieldData({}),

25
xmodule/x_module.py
View File

@@ -1239,31 +1239,6 @@ class ModuleSystemShim:
"""
self._deprecated_render_template = render_template
@property
def xqueue(self):
"""
Returns a dict containing the XQueueInterface object, as well as parameters for the specific StudentModule:
* interface: XQueueInterface object
* construct_callback: function to construct the fully-qualified LMS callback URL.
* default_queuename: default queue name for the course in XQueue
* waittime: number of seconds to wait in between calls to XQueue
Deprecated in favor of the xqueue service.
"""
warnings.warn(
'runtime.xqueue is deprecated. Please use the xqueue service instead.',
DeprecationWarning, stacklevel=3,
)
xqueue_service = self._runtime_services.get('xqueue') or self._services.get('xqueue')
if xqueue_service:
return {
'interface': xqueue_service.interface,
'construct_callback': xqueue_service.construct_callback,
'default_queuename': xqueue_service.default_queuename,
'waittime': xqueue_service.waittime,
}
return None
@property
def can_execute_unsafe_code(self):
"""