fix: Added xblock skill tag verification waffle switch - #33399
Conversation
7f46368 to
da49e6c
Compare
|
Is there an annotation for this waffle? Whenever a new waffle is added, it should be annotated following OEP-17. |
da49e6c to
f785826
Compare
f785826 to
e003217
Compare
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
| except VerticalBlockChildRenderStarted.PreventChildBlockRender as exc: | ||
| log.info("Skipping %s from vertical block. Reason: %s", child, exc.message) | ||
| continue | ||
| if XBLOCK_SKILL_TAG_VERIFICATION_SWITCH.is_enabled(): |
There was a problem hiding this comment.
@irfanuddinahmad: Why is the execution of the VerticalBlockChildRenderStarted and VerticalBlockRenderCompleted filters now gated behind XBLOCK_SKILL_TAG_VERIFICATION_SWITCH? The filters are supposed to be a general extension mechanism for people to override unit rendering behavior, and is not specific to one feature.
@tecoholic: As the original author that added the filters, please let me know if I'm reading this wrong.
FYI @nsprenkle, since this is an extension point you may want to use.
There was a problem hiding this comment.
@ormsbee Yes. VerticalBlockChildRenderStarted and VerticalBlockRenderCompleted are in fact meant to be generic filters. For eg., we use it in a course to automatically inject content like hyper-links to edit OLX source material stored in a Git repo like Gitlab/Github.
There was a problem hiding this comment.
@irfanuddinahmad: Also, when you're making a change like this, please put the reason why this change was done in the commit message. I'm assuming this was in response to some bug that was found, but I can't access that JIRA ticket to find out what it is or why it broke.
There was a problem hiding this comment.
@ormsbee ENT-7614 was in response to a downtime in Discovery and LMS due to delayed response originating from the XBlockSkillsAPI list endpoint that is responsible for gathering any skills linked to an xblock. This switch was added to quickly turn the feature on/off.
There was a problem hiding this comment.
@irfanuddinahmad: Thank you for the quick response. I think I understand what happened now.
This needs to get reverted. If 2U needs to be able to toggle off a plugin to make things more operationally robust, then the toggle and the conditional code should go inside that plugin. We absolutely cannot have this switch disable the entire filter pipelines that the plugin uses, since that will break everyone else who uses these extension points. The fact that this regression is hidden because the tests for it were altered to run with that switch enabled is also not an acceptable solution, since most people won't be running their instances that way.
This change made it into the Quince release. Per the release schedule, the next patch to Quince will be released on 2024-04-09.
Please have these changes reverted in master and back-ported into quince.master by 2024-04-02.
I want to give you folks the time to make a smooth transition here, but this is a breaking bug in Quince, and we need to get it patched.
Please reach out to me if you have any questions or concerns.
FYI @georgebabey: I'm making a request of your team here ⬆️.
Thanks folks.
7 participants
Description
This PR adds a waffle switch to enable/disable the xblock skill tag verification by students post their experience with the xblock.
Supporting information
JIRA: ENT-7614