-
Notifications
You must be signed in to change notification settings - Fork 15
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
TC_AUTHOR_60: STUDIO: CONTENT LIBRARIES: Library Sourced Content #231
Comments
label: olive testing |
label: release blocker |
I tested this in my local environment using tutor olive and got a similar error. I'll attach the error from the CMS logs:
I'm unfamiliar with blockstore so I don't know how to configure this. Can we confirm we're getting the same error in the testing instance? @regisb: can you help us check the logs? thanks! |
@DeanJayMathew, I think the Library Sourced block is exclusive for use with Blockstore. I believe we can let this one pass, as I don't think anybody intends to use Blockstore in production, specifically not with Tutor. |
Well noted, with thanks! If it's not going to be used, should we not remove the "Library Sourced Content" from the Open edX® standard distribution? Is there a formal procedure to allow/block these rarely-used features from making their way into the standard distribution? |
I can confirm that I'm seeing a similar error on the backend:
I can verify that the setting is None:
I have no idea why we are going through this particular piece of code when blockstore is not enabled. I'm taking the liberty to ping @bradenmacdonald, as the designated blockstore expert :) |
Using Library Sourced Block requires that you have Blockstore enabled. Blockstore now ships by default with Open edX so using it is merely a matter of setting the correct configuration, but it is not enabled out of the box. I am not sure why Library Source Content is enabled out of the box. I think the correct fix is one of these two things:
I would prefer the latter approach as it's more clear and also makes admins aware of the feature that they could enable if they wanted to. |
BTW here is where to add that check and there is already an example of how to display the error message. |
If blockstore is stable and provides useful features, then I think that we should configure Tutor to actually use it. What do you think? Is it worth it? If I understand correctly, correctly configuring Blockstore is just a matter of setting |
It looks like it's working! I'm a bit moved, it's the first time I see blockstore in action in Tutor :) All I had to do was to set What do you think, should we enable blockstore by default in Tutor? I think we should, if possible. |
Wow!
Let me know if we should create a Test Case for Olive and get it through
formal testing.
|
I guess? we should? I don't know because I have no idea what features are included in Blockstore... |
I don't consider myself a blockstore expert, but I have used it a lot in different contexts, and here's my take.
Blockstore is pretty stable and provides very useful features, but only if you can use it: it's just a storage backend. As far as I know, the only way to do so is via the Library Sourced block we're discussing, and it hasn't seen activity for some time. The last feature-meaningful commit was over 2 years ago., and even then we knew that it was just a placeholder for a better solution down the line. And (again, AFAIK) the only way to create content for the Library Sourced block is via frontend-app-library-authoring, and on that I can speak with some confidence: it's nowhere near being ready for production use.
Blockstore was designed to use object storage. Last I checked, the configuration of the store was separate. We'd have to check that this is the case, and then make it work with Minio, or expose corresponding Tutor config variables to the user. |
There is a PR to make it work with the (existing) "Randomized" Library Content Block too, but it has been waiting for review since August: openedx/edx-platform#30895 Yes, frontend-app-library-authoring is extremely immature. The visual editor is mostly not usable and could be a source of frustration to users who expect it to work. That said, the OLX authoring workflow works pretty well for advanced users.
Like the rest of edx-platform, blockstore will use django file storage by default, and you can change it to use S3/MinIO if you want. As you point out, right now that would mean Tutor would have to set an additional blockstore-specific config var to make it use MinIO. |
My 2c: "Library Sourced XBlock" only works with V2 libraries (the ones made by frontend-app-library-authoring, backed by blockstore). Blockstore is fine and stable, but V2 libraries are not. They cannot be authored using an instance running the default Tutor configuration. I think it'd be very confusing to have Library Sourced XBlock shown by default, when the underlying content it is exposing (V2 library blocks) cannot be authored by default. |
Relevant: openedx/edx-platform#30803 |
@arbrandes For the record, and contrary to what you said in the BTR meetup today, this is a new issue in Olive: the "library content" component button did not appear in Nutmeg, as you can see here: https://studio.demo.openedx.overhang.io/container/block-v1:edX+DemoX+Demo_Course+type@vertical+block@vertical_0270f6de40fc |
@regisb I think was the expected behavior in Nutmeg: https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/exercises_tools/randomized_content_blocks.html#enable-content-libraries . Before Olive, one needed to go into course Advanced Settings and add The "Library Content" button is a new thing in Olive ✨ |
@connorhaugh will look into this. |
I commented here: openedx/edx-platform#30803 (comment) tl;dr: we want to keep the "Randomized Content" but remove the "Library Source Content" xblock, right? Then I pushed a change to the Tutor settings to do just that. If this is an acceptable solution, then we can close this issue. |
Works for me! @DeanJayMathew, can you confirm? |
Ahh that works for me as well I submitted a PR to fix it on the olive branch, but Let's not be redundant. I missed this issue thread. I'll close that PR. openedx/edx-platform#31405 |
could you link me to your change? If we're in a rush then that works fine, but if we have the time to fix this upstream in edx-platform then IMO that'd be the best outcome. |
@regisb ^ |
@kdmccormick it's really the same patch as the one by @connorhaugh: https://github.com/overhangio/tutor/pull/734/files#diff-fc126cedbc6f2e010b4ccf860376e79778855985dab737cee6ff5b15f5bb0473 |
@connorhaugh would you be able to make this change at the source instead? That is, could you remove I think the strategy of "fix edx-platform's bad defaults at the Tutor layer" is OK as a stop-gap, but if we want edx-platform itself to be a good piece of software, then we need to get in the habit of fixing configuration issues at the source. |
Yes @arbrandes the following solution proposed by @regisb would resolve the issue from my perspective.
|
@connorhaugh has merged a fix to master and now has an Olive backport available for review: openedx/edx-platform#31424 |
The library_sourced block is not production-ready and thus should not be shown by-default in Studio. openedx/wg-build-test-release#231
Another release blocker bites the dust! Thanks everybody! |
Summary of Issue:
In Studio we have two options to add components from a Content Library, namely (1) Library Sourced Content and (2) Randomized Content Block:
I can create Libraries in Studio Home fine. From the two options mentioned above the "Randomized Content Block" works fine, but I get a error when trying the other option "Library Sourced Content":
Could not fetch library
SyntaxError: Unexpected token '<', " <!doctype "... is not valid JSON
Steps to reproduce:
3. Try to add a Library Sourced Component and click Edit to see the error. It also says an XBlock needs to be configured for this but I'm not sure which XBlock:
Expected Outcome: The Library Sourced Component should work.
Actual Outcome: It gives an error as described: Could not fetch library SyntaxError: Unexpected token '<', " <!doctype "... is not valid JSON
The text was updated successfully, but these errors were encountered: