-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Use runtime-provided XQueueService instead of constructing it in ProblemBlock #37998
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.
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
base: master
Are you sure you want to change the base?
Conversation
kdmccormick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the code looks good to me 👍🏻 please have another team member look at it too.
could you manually test this? To be honest, I'm not sure exactly how to set up testing for external grading, but I'll ask around.
| 'publish': EventPublishingService(user, course_id, track_function), | ||
| 'enrollments': EnrollmentsService(), | ||
| 'video_config': VideoConfigService(), | ||
| 'xqueue': XQueueService |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'xqueue': XQueueService | |
| 'xqueue': XQueueService, |
suggestion-- add a trailing comma
question-- will the runtime automatically construct this with the correct block parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how to test it, Still figuring it out. I was hoping that since 'i18n': XBlockI18nService, is supposed to be working and also has block as argument def __init__(self, block=None):, 'xqueue': XQueueService, should work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add print statements below the dictionaries and check if they added successfully.
Also see the logs while testing
|
Dave O tells me that @UsamaSadiq would probably be the most knowledgable person for how to manually test this. Usama, do you mind giving Irtaza pointers on how he could manually changes to the XQueueService class? I know that we're in the middle of migrating from the XQueue microservice to the in-process edx-submissions backend, and that both are currently supported. My understanding is that the XQueueService class delegates to either one, based on the waffle flag. So we should only need to test one of the backends (xqueue or edx-submissions) in order to validate this PR--whichever backend is easier to set up. |
|
I added some instructions to the PR description on how to test the XQueue Service locally using tutor dev and tutor local. I’ll follow up with @UsamaSadiq to discuss the testing details further. |
farhan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All seems good,
As per discussion please create a .rst document, how to test it and add its link in the class docs of XQueueService
| SEND_TO_SUBMISSION_COURSE_FLAG = CourseWaffleFlag("send_to_submission_course.enable", __name__) | ||
|
|
||
|
|
||
| class XQueueService: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As part of the ambition to remove xmodule, we should plan to move the Capa block code—that will stay reside in the platform—to a new dedicated location, either now or in a future phase.
We can create a new Django app inside core to house it.
| return block | ||
|
|
||
|
|
||
| @pytest.mark.django_db |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to add these declarations now and it was not needed before?
| 'publish': EventPublishingService(user, course_id, track_function), | ||
| 'enrollments': EnrollmentsService(), | ||
| 'video_config': VideoConfigService(), | ||
| 'xqueue': XQueueService |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add print statements below the dictionaries and check if they added successfully.
Also see the logs while testing
This PR updates
ProblemBlockto requestXQueueServicevia the XBlock runtime (self.runtime.service(self, "xqueue")) instead of directly constructing it inside the block and movesXQueueServicefromxmodule/capa/xqueue_interface.pytoxmodule/services.pyThis aligns with the standard service pattern and allows the Problem XBlock to live in xblocks-contrib/problem without copying the XQueueService implementation into that repo.
TESTING
OLX for sample question:
Mock Grader Script:
mock_grader.py
How to run this:
STEP 1: GET CREDENTIALS
Run this in your terminal:
tutor config printvalue XQUEUE_AUTH_PASSWORDSTEP 2: PREPARE SCRIPT
IN
mock_grader.py. Update theXQUEUE_PASSvariable with the password from Step 1.FOR TUTOR DEV SETUP:
STEP 3: COPY SCRIPT TO CONTAINER & EXECUTE SCRIPT
Run this command (replace
tutor_main_dev-lms-1with your actual LMS container name if different):docker cp mock_grader.py tutor_main_dev-lms-1:/tmp/mock_grader.pyRun this command to start the interactive grader:
tutor dev exec lms python /tmp/mock_grader.pyFOR TUTOR LOCAL SETUP:
STEP 3: EXECUTE SCRIPT
Run this command:
python mock_grader.pySTEP 5: TEST
Screen.Recording.2026-02-11.at.3.41.12.PM.mov
Related to #36538
Reference openedx/xblocks-contrib#151 (comment)