Skip to content
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

[DEPR]: Replace cs_comments_service #437

Open
6 of 7 tasks
regisb opened this issue Oct 2, 2024 · 17 comments
Open
6 of 7 tasks

[DEPR]: Replace cs_comments_service #437

regisb opened this issue Oct 2, 2024 · 17 comments
Assignees
Labels
depr Proposal for deprecation & removal per OEP-21

Comments

@regisb
Copy link

regisb commented Oct 2, 2024

Proposal Date

2024-04-16

Target Ticket Acceptance Date

2024-10-16

Earliest Open edX Named Release Without This Functionality

Sumac - 2024-10

Rationale

The Open edX forum is an essential feature of the platform. In technical terms, it is a web application that is developed in the cs_comments_service repository. This application is implemented in the Ruby programming language. Only a few Open edX contributors have expertise in Ruby: most developers are skilled in Python and NodeJS. This makes it extremely difficult to implement new features and extend the forum.

Moreover, the cs_comments_service application currently stores data in MongoDB, which is neither a consistent or a relational data storage solution -- as opposed to MySQL. This results both in operational and architecture complexity, as some data needs to be synced up between the two data stores (notably for user data).

Removal

The cs_comments_service repository will be removed. Note howerver that the tutor-forum plugin will be maintained, though it will be considerably simplified.

Replacement

The Ruby application will be replaced by a Django app, currently being developed in the following repository: https://github.com/edly-io/forum/ (soon to be migrated to the openedx organization, as per this public-engineering issue).

Deprecation

The cs_comments_service repository will include a deprecation notice in its README file. Migration will be transparent for most users running Tutor, as the tutor-forum plugin will smoothly transition from running the former ruby app to installing the new forum application in the edx-platform runtime.

Migration

Strategy

Let's call the legacy cs_comments_service "forum v1" and the new forum application "forum v2".

In forum v1, edx-platform communicates with the Ruby application via an HTTP rest API. All calls are made from the backend, Client-side API calls are made to edx-platform, which then forwards parts of the requests to forum v1.

In forum v2, these API calls take the form of a native Python API. This is made possible by the fact that the forum v2 application is installed in the same runtime as edx-platform.

image

This means that the diagram above will remain the same in the transition from v1 to v2. The only difference is that in step 3, communication will change from an HTTP API call to a native Python function call.

For storage, forum v2 will implement two different backends: one for MySQL (in the form of Django models) and one for MongoDB. The MongoDB backend is only preserved to ease the transition from v1 to v2; it is expected that it will be eventually removed.

Opt-out

Forum v2 will make use of two CourseWaffleFlag (source code). For every course, platform administrators will have the choice to:

  1. Send HTTP requests to the forum v1 app or native API calls to the forum v2 app.
  2. If calling the forum v2 app, use either the legacy MongoDB backend or the new MySQL backend.
  3. Run a dedicated migration command that will copy the course data from MongoDB to the new MySQL models.

By default, users running Tutor (and the tutor-forum plugin) will automatically transition from v1 to v2 and the MySQL models. In that sense, the transition is opt-out rather than opt-in.

Proposed timeline

  • July - October 23rd 2024: implementation of forum v2
  • October 23rd 2024: Forum v1 marked for deprecation: no new change accepted in the cs_comments_service repository.
  • October 23rd - December 9th 2024: Sumac testing, fixing issues in forum v2.
  • December 9th 2024: Sumac release
    • By default, Tutor will run forum v2. The storage backend will depend on whether or not it's a new tutor installation
      • Existing platforms will keep using MongoDB: instructions to migrate will be printed on tutor ... launch and be available in the tutor-forum README.
      • New platforms will default to the MySQL backend.
    • It will be possible to override this default behavior on a course-per-course basis.
  • December 2024 - April 2025: all platforms are expected to migrate from forum v1 to forum v2, with the MySQL storage backend.
  • June 2025: Teak release:
    • the forum v1 integration is removed from edx-platform
    • the mongodb backend is removed from forum v2

Additional Info

Task List

@github-actions github-actions bot added the depr Proposal for deprecation & removal per OEP-21 label Oct 2, 2024
@kdmccormick
Copy link
Member

It sounds like the "Earliest Open edX Named Release Without This Functionality" will actually be Teak, not Sumac?

@regisb regisb changed the title [DEPR]: cs_comments_service replacement [DEPR]: Replace cs_comments_service Oct 3, 2024
@regisb
Copy link
Author

regisb commented Oct 3, 2024

@kdmccormick Yes, I wasn't sure to indicate Sumac or Teak in this field. We are proposing that cs_comments_service be removed in Teak but, technically speaking, Sumac will be the first release to ship with the replacement of cs_comments_service by default. In the end I decided to announce the change for Sumac to avoid taking people by surprise.

@ormsbee
Copy link

ormsbee commented Nov 11, 2024

@hurtstotouchfire: Do you folks have time to give some feedback on this proposal?

Thank you.

@feanil feanil moved this from Proposed to Accepted in DEPR: Deprecation & Removal Nov 13, 2024
@feanil feanil assigned feanil and regisb and unassigned feanil Nov 13, 2024
@regisb
Copy link
Author

regisb commented Nov 14, 2024

I'll recap here the decisions that were reached in the BTR working group meeting that happened yesterday:

  • Forum v2 support will be merged in edx-platform in Sumac.
  • Platforms not running Tutor will keep running forum v1 by default.
  • Platform running Tutor will run forum v2, but with different data backends:
    • Existing platforms will keep using MongoDB: instructions to migrate will be printed on tutor ... launch and be available in the tutor-forum README.
    • New platforms will default to the MySQL backend.
  • Platforms running Tutor and which need to stick to forum v1 will have to create their own Tutor plugin, likely based on top of this version.
  • Legacy forum v1-related code will be removed from edx-platform before Teak.
  • Legacy MongoDB related code will be removed from forum v2 before Teak, with the exception of the migration script, which will remain available in Teak.

@feanil
Copy link
Contributor

feanil commented Nov 14, 2024

I've updated the timeline in the description to match this recap.

@jristau1984
Copy link

Hello! Can a separate DEPR ticket be created for the MongoDB portion of what is listed in the description here? It feels like something independent (at least from a timeline perspective) of cs_comments_service. Thanks!

@kbuchanan-2u
Copy link

Hi guys, it does not look like Forums V2 is actually merged and working for Sumac at this point. Can we update the DEPR timelines to reflect this?

@regisb
Copy link
Author

regisb commented Dec 9, 2024

@jristau1984 You mean to say that the migration of data from mongodb to mysql should be migrated to a separate DEPR ticket? What for? Would you like to have a different timeline?

@kbuchanan-2u Forum v2 was merged and is working in Sumac. It was rollbacked by 2U in master, but the release branch is doing just fine.

@ayub02
Copy link

ayub02 commented Dec 12, 2024

@regisb Would you be able to share the test results for this work? From product perspective, I'm curious to know which forum features and workflows the team has considered.

@regisb
Copy link
Author

regisb commented Dec 12, 2024

I assume you mean Q&A testing, right @ayub02? (Forum v2 has extensive unit and integration testing, but I suppose you are not referring to those)

The following scenarios were tested:

  • Integration of edx-platform with forum v2:
    • With mysql backend
    • With mongodb backend
    • With both mongodb+mysql backends for different courses
    • With Elasticsearch as a search backend
    • With Meilisearch as a search backend
  • Compatibility of forum v1 in edx-platform:
    • By using forum v1 exclusively
    • By using forum v2 exclusively
    • By using forum v1 and v2 for different courses
  • Data migration from mongodb to mysql: making sure that data is preserved and compatible with forum v2.

Those scenarios were tested in the context of a Tutor deployment. As a consequence, some differences with the 2U architecture were not detected, including:

  • The presence of incompatible mongodb indices
  • Different MongoDB hostnames

Our team was composed exclusively of developers and did not include Q&A members, so these results were not properly collected in a testing sheet. Still, I can guarantee that these tests were conducted.

Do you have such a test sheet that you could share with us? It would be quite useful both to us and to the build/test/release working group, which is also testing the forum for every major Open edX release.

@ayub02
Copy link

ayub02 commented Dec 13, 2024

@regisb yes i was asking about Q&A testing. Thank you for sharing the results. My question is more about user acceptance testing (UAT). Forums unfortunately has one of the most complex workflows in edX. Here is a sheet that we have used in the past to test discussion MFE after the completion of BD-38. https://docs.google.com/spreadsheets/d/1YEr9Mv6jTUmDNoz3GyZv0vIYuL3CARi6/edit?usp=sharing&ouid=111742414635622912267&rtpof=true&sd=true

Maybe your team and run these tests with a backend configuration(s) that makes sense (you and @asadazam93 can probably discuss that).

@feanil feanil moved this from Accepted to Blocked in DEPR: Deprecation & Removal Jan 23, 2025
@feanil
Copy link
Contributor

feanil commented Jan 23, 2025

Marking as blocked until we can remove the old compatibility layer.

@dianakhuang dianakhuang moved this from Blocked to Ready for Migration in DEPR: Deprecation & Removal Jan 23, 2025
@kbuchanan-2u
Copy link

kbuchanan-2u commented Jan 31, 2025

FYI we have logged the following issues:
openedx/forum#142
openedx/forum#144
openedx/forum#147
openedx/forum#149
openedx/forum#150
openedx/forum#151
openedx/forum#152
openedx/forum#153

Many of these issues block additional testing because they are the base cases for functionality that still has additional cases to test. Because of this we have not completed our testing. Most of these issues would block us from fully migrating edx.org onto Forums V2.

I want to also raise that for openedx/forum#149 in particular we have really no timeline for remediating this. We recognize that the xblock is unsupported, but we never completed the process of deprecating it on edx.org because it is enabled for certain long-running course runs, and there is no migration path without starting a new course run. We are working on figuring out a path forward for this edge case, but none of our options will be quick.

@ormsbee
Copy link

ormsbee commented Jan 31, 2025

@kbuchanan-2u: Thank you for the update, especially in regards to openedx/forum#149. I'm trying to find the proper DEPR trail for both the old non-MFE interface for forums as well as the inline discussion blocks. It looks like the ENABLE_DISCUSSIONS_MFE was intended to be removed by 2022-12-05, but it's still in there.

Let's continue this discussion next week.

@ormsbee
Copy link

ormsbee commented Feb 3, 2025

@kbuchanan-2u:

We recognize that the xblock is unsupported, but we never completed the process of deprecating it on edx.org because it is enabled for certain long-running course runs, and there is no migration path without starting a new course run. We are working on figuring out a path forward for this edge case, but none of our options will be quick.

Yeah, it does seem to be in a weird state right now, since new courses can't even create inline discussion blocks, and yet there's no formal deprecation of that code. In terms of being "unsupported"–the XBlock in question still works for old courses and hasn't been deprecated anywhere as far as I can tell, so I believe that as far as the project is concerned, it's still supported.

So this is my understanding of the situation around openedx/forum#149 as a whole:

  1. The non-MFE tab view of forums was never deprecated, and continues to work for old courses.
  2. The inline discussion XBlock was never deprecated, and continues to work for old courses.
  3. Both (1) and (2) rely on older forums API endpoints that were never ported to v2 forums because those clients were presumed to be deprecated.

If any of these are incorrect, then please let me know, because nothing after this sentence is going to make sense in that case.

I'm going to assume that creating a DEPR for the non-MFE forums tab view is not controversial. I can do this.

In terms of the inline XBlock, I can think of a few options:

A: Deprecate the inline XBlock. This would be the least amount of development effort, though it means that the v2 forum switchover would have to be pushed out a release cycle in order to give this new DEPR time to run. I personally favor this.

B: Create a data migration. Create some automated way to convert old inline discussion blocks to their modern Unit-sidebar equivalents. I realize they're not semantically identical, but given common practices are to put one inline discussion per Unit, it's possible that something like this could work for the majority of cases. However, we've generally avoided doing things that force content publishing, since that carries many hazards with it. This is the most operationally risky path.

C: Modify the inline discussion XBlock to use the new v2 endpoints. I'm not sure what the scope of this would be, and it seems wasteful to be throwing a lot of effort into backwards compatibility with something that was turned off in 2023. Still, this would unblock the v2 forums migration for Teak.

D: Support the old endpoints using v2 forums. Similar to (C) in spirit and unblocking v2 forums migration for Teak, but carrying around slightly more legacy baggage. Maybe preferable if there is other code out there also hitting these endpoints (old mobile app code?) Again, I don't know the extent of this effort.

Am I missing anything?

@ayub02
Copy link

ayub02 commented Feb 4, 2025

@ormsbee thank you for thinking through this. I want to clarify a few things:

  1. There’s been a misunderstanding about discussion xblocks. 2U does not and never intended to deprecate discussion xblocks. This old repo is causing this confusion and has nothing to do with discussion xblocks that are being used in 2U and by community: https://github.com/openedx-unsupported/xblock-discussion. Here’s is the link to the discussion xblock code currently in use: https://github.com/openedx/edx-platform/blob/f9126bfdd990dc78c97d10619bd2b08ad47a1a18/xmodule/discussion_block.py#L39
  2. Our understanding is that forum v2 is a replacement for cs_comment_service. Which means that it needs to support everything that cs_comment_service supports at present, including discussion xblocks.
  3. The image below illustrates the impact of flags on overall discussions experience. Note that the non-MFE UI and discussion xblocks are default experiences for community at present.
  4. Rollout doc for discussions MFE experience: https://openedx.atlassian.net/wiki/spaces/COMM/pages/3509551260/New+discussions+forum+experience (explanation of some features may be outdated)
  5. Rollout doc for "openedx" discussion provider (a.k.a discussion sidebar): https://openedx.atlassian.net/wiki/spaces/COMM/pages/3470655498/Discussions+upgrade+Sidebar+and+new+topic+structure (explanation of some features may be outdated)

The only way forward that makes sense to me is fixing forum v2 to work with discussion xblocks.

Also, i'll avoid using the word "legacy" in conversations because it may give a false indication of a pending deprecation. Except that the old discussion provider was renamed to "legacy" by OpenCraft during BD-38 ("old" for 2U, not for community).

I hope this helps.

Image

@ormsbee
Copy link

ormsbee commented Feb 4, 2025

@ayub02: Thank you for the quick reply.

  1. There’s been a misunderstanding about discussion xblocks. 2U does not and never intended to deprecate discussion xblocks. This old repo is causing this confusion and has nothing to do with discussion xblocks that are being used in 2U and by community: https://github.com/openedx-unsupported/xblock-discussion. Here’s is the link to the discussion xblock code currently in use: https://github.com/openedx/edx-platform/blob/f9126bfdd990dc78c97d10619bd2b08ad47a1a18/xmodule/discussion_block.py#L39

I understood the situation with that old discussion block repo when I wrote my comment, and the link I wrote there was to the one in edx-platform. That is not the reason I thought that inline discussion blocks were intended for deprecation. Instead, the reasons are:

  • The inline discussion block is referred to in a number of places as part of the "legacy" experience, both in the rollout docs, and in the user documentation.
  • The rollout doc you link explicitly says that, "These xblocks will eventually be replaced with the discussion sidebar." (emphasis mine).
  • Re-runs of courses ignore inline discussion blocks, hides them from learners, and prompts a warning banner at the top of Studio reading, "This course run is using an upgraded version of {platform name} discussion forum. In order to display the discussions sidebar, discussions xBlocks will no longer be visible to learners."
  • The option to add inline discussion blocks was removed from the default experience. Authors can't even opt into the old way–they need operator/support intervention.

Inline discussion blocks represent an old way of providing in-context discussion capability at the unit level. It has been documented and advertised as a legacy feature since 2023, when it was replaced by the new sidebar experience. From a product point of view, this is clearly a de facto deprecation. That's why I think @kbuchanan-2u's take is the correct one here:

We recognize that the xblock is unsupported, but we never completed the process of deprecating it on edx.org because it is enabled for certain long-running course runs, and there is no migration path without starting a new course run.

It's also worth noting that the ENABLE_NEW_STRUCTURE_DISCUSSIONS flag referenced in the diagram was intended to be removed on 2022-12-22. Again, pointing to an intended switchover that stalled out at some point.

  1. Our understanding is that forum v2 is a replacement for cs_comment_service. Which means that it needs to support everything that cs_comment_service supports at present, including discussion xblocks.

The goal of the v2 forums rollout is to improve maintainability by aligning forums with the rest of our development stack, i.e. Django app + ORM backend, vs. Ruby service + MongoDB. In the long term, this will reduce operational and code complexity.

Part of the simplification process involves removing features that are not in use today. For instance, the original forums data model allowed for arbitrary nesting of comment replies. Very early on, the product team decided against this direction and moved to the post-comment-reply scheme that we have now. There is no need for v2 forums to support arbitrary nesting of comments, since the only courses that ever ran in this mode used a Modulestore that no longer exists, and therefore can no longer be accessed by anyone.

This happens in other places as well. XQueue still has code for push-grading, where XQueue itself would make direct HTTP calls to graders. This was an operational nightmare, and the reason why everyone switched over to using the pull mode and xqueue-watcher. The XQueue deprecation aims to be fully backwards compatible with xqueue-watcher, but not push-grading.

Figuring out what does and doesn't need to be supported during a migration of this sort is what these DEPR ticket conversations are for. I realize that the inline discussion blocks are still in use today, but if they were deprecated and removed, there would be no need for v2 forums to support them.

The image below illustrates the impact of flags on overall discussions experience.

The image is hugely helpful, thank you.

Note that the non-MFE UI and discussion xblocks are default experiences for community at present.

In terms of configuration, most of the community either runs Tutor (which sets the flags to run the newer discussions setup), or is run by operators who keep up with the latest. This mismatch between the base settings and what everyone actually runs has been a long standing issue that we're trying to reduce over time, but sometimes it's the only practical short-term workaround when dealing with deployment flags that are the de facto (tested-and-run) standard.

So I think most of the community that is on later releases does run the new code path, which likely means that there are a number of bugs accumulating when trying to run the non-MFE version of the forums with the latest master. Which brings us back to one of the main reasons we have the DEPR process.

The only way forward that makes sense to me is fixing forum v2 to work with discussion xblocks.

I can see this making sense if it's a relatively light lift on the dev side, since it will unblock a Teak rollout for v2 forum switchover. But I want to emphasize that I still consider inline discussion blocks to be on the path to deprecation. If we adapt v2 forums code to accommodate inline discussion blocks, it will only be to decouple them from this DEPR and its timeline.

I can take a first pass at writing the DEPR for inline discussion blocks. I realize that there's a lot going on right now and that the migration portion of that DEPR may get complicated. They will absolutely still be supported in Teak–any removal wouldn't happen until at least the Ulmo release cycle. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
depr Proposal for deprecation & removal per OEP-21
Projects
Status: Replacement Ready
Development

No branches or pull requests

7 participants