search

openedx / frontend-app-authoring

Front-end for the Open edX Studio experience, implemented in React and Paragon.
GNU Affero General Public License v3.0
13 stars 76 forks source link

Default Configurations for new libraries in Sumac #1334

Open jmakowski1123 opened 3 weeks ago

jmakowski1123 commented 3 weeks ago

The new Libraries feature will be turned on by default for all new Sumac instances (if Meilisearch is available). Elements of the new feature will be marked with a "beta" label in the interface. There should be an option to opt out of the new Libraries feature by site operators at the instance level.

Legacy Libraries will remain supported and unchanged in Sumac. Anyone who wishes to use Legacy Libraries can manually configure it via the "library_content" json string in Advanced Settings.

Any instance that chooses to continue using Legacy Libraries will have both Library experiences on.

We will message the following rollout plan in the Sumac documentation:

jmakowski1123 commented 3 weeks ago

cc @kdmccormick @bradenmacdonald @ormsbee re: complexities of Tutor settings

cc @cablaa77 to ensure 2U needs are met here

pomegranited commented 3 weeks ago

The new Libraries feature will be turned on by default for all new Sumac instances (if Meilisearch is available). ... There should be an option to opt out of the new Libraries feature by site operators at the instance level.

@bradenmacdonald We'll need to modify tutor-mfe to support making the Authoring MFE's LIBRARY_MODE default dependent on Meilisearch.

And we should get https://github.com/open-craft/tutor-contrib-meilisearch moved to openedx, and listed in Sumac's plugins list.

I'm also looking at tutor's docs for upgrading between releases -- Should we add prompts for operators upgrading to Sumac to recommend adding the Meilisearch plugin? Or is relying on release documentation enough?

Anyone who wishes to use Legacy Libraries can manually configure it via the "library_content" json string in Advanced Settings.

Pushing back here -- operators will have to allow legacy libraries in the Authoring MFE during the upgrade, we can't let course authors toggle it on/off.

bradenmacdonald commented 3 weeks ago

@pomegranited We'll probably also have a waffle flag involved, so I think it should be the CMS (via the "studio home API") and not Tutor which tells the MFE which library mode to use. That way 2U can roll it out on a per-user basis later.

And we should get https://github.com/open-craft/tutor-contrib-meilisearch moved to openedx, and listed in Sumac's plugins list.

It may make more sense to have it in tutor core. We want it available by default for everyone unless they really want to opt out. I'll let Regis decide if that makes it core or plugin. AFAIK elasticsearch support is in tutor core, not a plugin. In any case, it can happen after the cut as part of the release process.

operators will have to allow legacy libraries in the Authoring MFE during the upgrade, we can't let course authors toggle it on/off.

There's two different things. Using legacy libraries in a course will still require 'manually configure it via the "library_content" json string in Advanced Settings.' as it always has. Accessing the legacy library itself in Studio will be available to everyone via studio home just as it currently is.

pomegranited commented 3 weeks ago

Thanks @bradenmacdonald -- I've continued the discussion with you and Jeremy on https://github.com/openedx/frontend-app-authoring/pull/1329#issuecomment-2377499537

kdmccormick commented 3 weeks ago

Using legacy libraries in a course will still require 'manually configure it via the "library_content" json string in Advanced Settings.'

(Legacy) Library Content content is enabled out-of-the box for courses since at least Redwood, possibly Quince.

bradenmacdonald commented 3 weeks ago

Oh, right.

kdmccormick commented 2 weeks ago

(Copied in from https://github.com/openedx/frontend-app-authoring/pull/1329#issuecomment-2377499537):

Just confirming for all parties involved: There will be two new Waffles:

  1. contentstore.new_studio_mfe.disable_legacy_libraries
  2. contentstore.new_studio_mfe.disable_new_libraries

In order to keep the current (legacy-only) experience, operators will need to force-on waffle (2).

jmakowski1123 commented 2 weeks ago

@bradenmacdonald Just saw your additional comment about meilisearch - is this a contingency to be concerned about?

bradenmacdonald commented 2 weeks ago

@jmakowski1123 No; on all small instances, I think it will be available (thanks to Tutor). It's only very large instances where Meilisearch may not be available (yet - because they will need to do a bit more work to set it up the way that they want), and in that case I just want to avoid displaying error messages or a broken experience to users. Does that make sense?

regisb commented 2 weeks ago

Responding to @pomegranited's Slack comment here:

The Modular Learning project would like to enable the new Libraries feature in Authoring MFE by default for Sumac. This requires running Meilisearch, which is a Tutor contrib plugin available since Redwood: https://github.com/open-craft/tutor-contrib-meilisearch

Enabling Meilisearch also allows content authors to search draft course content in the Authoring MFE.We'd like to make Tutor install and configure Meilisearch by default, because this is the easiest way for small/medium instances to use this feature. Very large instances (like MIT and 2U) will need to do some more operational work to run Meilisearch, and so we're adding waffle flags.

What is the best way to support these changes for Sumac? Specifically:

  1. Should tutor-contrib-meilisearch remain a separate plugin (and moved to the main tpi list), or should it be integrated into Tutor core (like Elasticsearch)?
  2. If we keep the plugin separate, do we need to change ownership to the openedx org?

I'm excited about this change, and I think that the meilisearch plugin should be merged in Tutor core.

BUT to make meilisearch a mandatory part of Tutor, we should get rid of Elasticsearch. That's because Meilisearch and Elasticsearch fulfill identical roles, and running both requires extra server resources.

I understand this is a big requirement. To achieve that we will need to:

  1. Finalize and merge these PRs: a. https://github.com/openedx/edx-platform/pull/35177 b. https://github.com/openedx/frontend-app-learning/pull/1433
  2. Migrate openedx-search-api to the openedx organization.
  3. Make the new forum compatible with Meilisearch.

Again, I understand that this is a big endeavour. That's just my opinion, and I'm curious to hear from the other folks in the BTR.

bradenmacdonald commented 2 weeks ago

@regisb

BUT to make meilisearch a mandatory part of Tutor, we should get rid of Elasticsearch. That's because Meilisearch and Elasticsearch fulfill identical roles, and running both requires extra server resources.

I'm all for getting rid of Elasticsearch ASAP. And it's true they fullfill identical roles. But I do want to note that Meilisearch hardly requires any extra resources at all. On my devstack it uses under 30MB RAM and minimal CPU. Here you can see its usage when I performed a search:

Screenshot

Based on this, I don't believe anyone will be substantially affected by the increase resource usage.

I think I have been struggling with understanding the context of these PRs because they don't have a lot of descriptive writing (README, PR descriptions, ADRs). That makes them hard to review. It would be helpful if someone could work with the author to explain the intention of each PR in more detail, as well as the design of the openedx-search-api, and especially the implications for the existing systems and how this will affect them. For example, what will happen to our existing studio search UI and studio content search backend - will that exist alongside opendx-search-api or does it need to be rewritten?

I also have a lot of questions/concerns about the opendx-search-api. In short, it's only accounting for the absolute most basic use case, and it doesn't account for the much more complex Studio use cases we've had over the past year.

We are getting very close to the Sumac cut. Given the short time, I think it would be possible for someone to achieve:

I don't think it will be possible to finish the design and implementation of a future-proof abstraction layer such as openedx-search-api aims to be. It will require so much more work than has happened to date. I explained some of the next steps on https://github.com/edly-io/openedx-search-api/issues/19

pomegranited commented 1 week ago

@bradenmacdonald @regisb Thank you for discussing this issue :)

We are getting very close to the Sumac cut. Given the short time, I think it would be possible for someone to achieve:

  • Support Meilisearch as a backend for the LMS courseware search, instead of ElasticSearch (for Sumac you can choose which backend to deploy). This would use the existing edx-search API and be a fairly basic use case.
  • And, get the new forum to work with Meilisearch.

"Someone" might be able to do this, but I don't think we can, given everything else that's required for Modular Learning in Sumac.. 😟

BUT to make meilisearch a mandatory part of Tutor, we should get rid of Elasticsearch.

@regisb I agree completely, but I'm hoping we can find a middle ground. What if we:

  1. Enable Meilisearch by default in Sumac, but ensure that it (and the features it supports) can be disabled without causing problems for learners & course authors.
  2. Deprecate Elasticsearch with Sumac.
  3. Commit to removing Elasticsearch in Teak by converting the remaining Elasticsearch dependencies and providing migration pathways

Would that be enough to let us enable Meilisearch in Sumac by default?

@bradenmacdonald can we commit to the above?

bradenmacdonald commented 1 week ago

@pomegranited (1) we can commit to; that's definitely the goal. As for (2) and (3) it's not my call to make :P

jmakowski1123 commented 1 week ago

I believe the long-term plan is to deprecate elasticsearch, but not on a sumac timeline. Copying in @ormsbee .

regisb commented 1 week ago

It took me a while to reply because I wanted to try out a proof of concept.

At Edly, our initial plan was to propose a replacement by default for Elasticsearch in the form of openedx-search-api. This repo would have replaced edx-search. This project has had setbacks, which means that we are not in a position to propose an improved search API that would be compatible both with Elasticsearch and Meilisearch.

That being said, we really really want to get rid of Elasticsearch in Sumac. So what I did is that I started working on a search engine that is compatible with the edx-search API, but that connects with Meilisearch instead of Elasticsearch. If we achieve that, then replacing Elasticsearch by Meilisearch within edx-platform will be as simple as installing an extra dependency (or making a PR to edx-platform if we decide that the engine should live there) and then change the SEARCH_ENGINE setting. My initial research is very promising, and I think I can propose a Meilisearch-compatible engine in time for the Sumac release. This engine will cover the following features:

And then studio search would be covered by the engine developed by Braden.

This would mean that Tutor would no longer ship with Elasticsearch by default, but only with Meilisearch. 2U and others would still be able to run Elasticsearch by keeping their default settings.

We would then take the time to decide whether we want to DEPR Elasticsearch, at our leisure.

Would that be an acceptable solution?

bradenmacdonald commented 1 week ago

I support that plan ^

pomegranited commented 1 week ago

That sounds amazing @regisb ! I have CC time available for reviews, testing, and/or release documentation, please let me know how I can help?

regisb commented 5 days ago

Thanks a lot for the help @pomegranited! I opened a PR here: https://github.com/openedx/edx-platform/pull/35650

pomegranited commented 3 days ago

@jmakowski1123 @bradenmacdonald CC @kdmccormick Bumping this question from https://github.com/openedx/frontend-app-authoring/pull/1329#issuecomment-2389088972 :

Right now there is a feature flag called ENABLE_CONTENT_LIBRARIES. It defaults to True (legacy libraries on) but some site operators override it to False (legacy libraries off).

For those site operators who have set it to False, what should happen with New Libraries in Sumac? Should they see the just the New Libraries experience, or should all Libraries features remain disabled?

bradenmacdonald commented 3 days ago

@pomegranited Personally I would expect v2 libraries are also off in that scenario.

jmakowski1123 commented 3 days ago

I'd go conservative for Sumac, since Libraries is still in beta, and say if it's set to False then the new libraries should also be off. But once we deprecate legacy libraries, I think instance operators should need to make a new decision about setting the new libraries to off, given that it is really a different feature set entirely from legacy libraries.

pomegranited commented 3 days ago

Thanks @jmakowski1123 !

But once we deprecate legacy libraries, I think instance operators should need to make a new decision about setting the new libraries to off

When we're ready for that step, we can point people to these added waffle flags, and remove that FEATURES['ENABLE_CONTENT_LIBRARIES'] flag entirely. But that can wait :)

pomegranited commented 3 days ago

@regisb

I opened a PR here: https://github.com/openedx/edx-platform/pull/35650

Thank you! I think this gives us a plausible start towards replacing Elasticsearch with Meilisearch in Tutor core. Assuming that makes it into Sumac, are you good with making Meilisearch a "core plugin" that's enabled by default? What's the best way to approach this?

regisb commented 3 days ago

@pomegranited Yes, tutor-contrib-meilisearch should be merged in Tutor core in Sumac. We need to improve it a little, by including a k8s deployment, the possibility to connect to a remote instance, etc. but given how easy it is to deploy I don't expect any major difficulty.

pomegranited commented 1 day ago

@regisb

Yes, tutor-contrib-meilisearch should be merged in Tutor core in Sumac. We need to improve it a little, by including a k8s deployment, the possibility to connect to a remote instance, etc. but given how easy it is to deploy I don't expect any major difficulty.

Created https://github.com/openedx/modular-learning/issues/236 for this.