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

[sumac] feat: migrate from Ruby to forum v2 Python app #48

Merged
merged 1 commit into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
variables:
TUTOR_PLUGIN: forum
TUTOR_IMAGES: forum
TUTOR_PYPI_PACKAGE: tutor-forum
GITHUB_REPO: overhangio/tutor-forum

Expand Down
1 change: 1 addition & 0 deletions changelog.d/20241017_163150_regis_forumv2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- 💥[Feature] Switch from the legacy `cs_comments_service` Ruby app to the new forum v2 Python app. In addition, forum data is now stored in MySQL, and not in MongoDb. This considerably simplifies this plugin. Change should be transparent for most users, unless the forum backend has been customised in some way. This is considered a breaking change because the legacy forum will no longer be supported by this plugin. Users who want to keep running `cs_comments_service` will have to create their own plugin. (by @regisb)
11 changes: 0 additions & 11 deletions tutorforum/hooks.py

This file was deleted.

26 changes: 0 additions & 26 deletions tutorforum/patches/k8s-deployments

This file was deleted.

16 changes: 0 additions & 16 deletions tutorforum/patches/k8s-jobs

This file was deleted.

12 changes: 0 additions & 12 deletions tutorforum/patches/k8s-services

This file was deleted.

2 changes: 0 additions & 2 deletions tutorforum/patches/lms-env

This file was deleted.

3 changes: 0 additions & 3 deletions tutorforum/patches/local-docker-compose-dev-services

This file was deleted.

12 changes: 0 additions & 12 deletions tutorforum/patches/local-docker-compose-jobs-services

This file was deleted.

1 change: 0 additions & 1 deletion tutorforum/patches/local-docker-compose-lms-dependencies

This file was deleted.

13 changes: 0 additions & 13 deletions tutorforum/patches/local-docker-compose-services

This file was deleted.

1 change: 0 additions & 1 deletion tutorforum/patches/openedx-common-settings

This file was deleted.

1 change: 0 additions & 1 deletion tutorforum/patches/openedx-lms-development-settings

This file was deleted.

232 changes: 29 additions & 203 deletions tutorforum/plugin.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
from __future__ import annotations

import os
import urllib.parse
from glob import glob

import importlib_resources
from tutor import hooks as tutor_hooks
from tutor.__about__ import __version_suffix__

from .__about__ import __version__
from .hooks import FORUM_ENV

# Handle version suffix in nightly mode, just like tutor core
if __version_suffix__:
Expand All @@ -18,213 +12,45 @@
config = {
"defaults": {
"VERSION": __version__,
"DOCKER_IMAGE": "{{ DOCKER_REGISTRY }}overhangio/openedx-forum:{{ FORUM_VERSION }}",
"MONGODB_DATABASE": "cs_comments_service",
"PORT": "4567",
"API_KEY": "forumapikey",
"REPOSITORY": "https://github.com/openedx/cs_comments_service.git",
"REPOSITORY_VERSION": "{{ OPENEDX_COMMON_VERSION }}",
},
}

FORUM_ENV_BASE: dict[str, str] = {
"SEARCH_SERVER": "{{ ELASTICSEARCH_SCHEME }}://{{ ELASTICSEARCH_HOST }}:{{ ELASTICSEARCH_PORT }}",
"MONGODB_AUTH": "{{ get_mongo_auth(MONGODB_USERNAME, MONGODB_PASSWORD) }}",
"MONGODB_HOST": "{{ MONGODB_HOST|forum_mongodb_host }}",
"MONGODB_PORT": "{{ MONGODB_PORT }}",
"MONGODB_DATABASE": "{{ FORUM_MONGODB_DATABASE }}",
"MONGOID_AUTH_SOURCE": "{{ MONGODB_AUTH_SOURCE }}",
"MONGOID_AUTH_MECH": "{{ MONGODB_AUTH_MECHANISM|auth_mech_as_ruby }}",
"MONGOID_USE_SSL": "{{ 'true' if MONGODB_USE_SSL else 'false' }}",
"MONGOHQ_URL": "{{ get_mongohq_url(MONGODB_HOST, MONGODB_PORT,MONGODB_DATABASE, get_mongo_auth(MONGODB_USERNAME, MONGODB_PASSWORD)) }}",
}

with open(
str(
importlib_resources.files("tutorforum")
/ os.path.join("templates", "forum", "tasks", "forum", "init")
),
encoding="utf8",
) as f:
tutor_hooks.Filters.CLI_DO_INIT_TASKS.add_item(("forum", f.read()))
# Auto-mount forum repository
tutor_hooks.Filters.MOUNTED_DIRECTORIES.add_item(("openedx", "forum"))

tutor_hooks.Filters.IMAGES_BUILD.add_item(
(
"forum",
("plugins", "forum", "build", "forum"),
"{{ FORUM_DOCKER_IMAGE }}",
(),
)
)
tutor_hooks.Filters.IMAGES_PULL.add_item(
(
"forum",
"{{ FORUM_DOCKER_IMAGE }}",
)
tutor_hooks.Filters.ENV_PATCHES.add_items(
[
# Patch edx-platform
# https://github.com/openedx/edx-platform/pull/35671
# TODO after this PR has been merged, remove this patch
(
"openedx-dockerfile-post-git-checkout",
"""
RUN git remote add edly https://github.com/edly-io/edx-platform \
&& git fetch edly edly/forumv2 \
&& git merge edly/edly/forumv2""",
),
Comment on lines +29 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying/merging that path caused an error on tutor sumac branch: 255.9 fatal: refusing to merge unrelated histories

May be edx/forumv2 brnach needs to be rebased on top of open-release/sumac.master?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright I think the above error is because tutor makes a shalllow clone, so I tried 1) git merge edly/edly/forumv2 --allow-unrelated-histories, but then there is a huge a merge conflcit. So then I noticed edly branch is quite update to with master/sumac, so I tried git checkout edly/edly/forumv2 but then there I got pypi dependency conflict

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let you know after updating(resolving dependency conflict) the edly/forum branch with upstream.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dependency Issues were resolved, forgot to add it here as well. You can test it...

# Enable forum feature
(
"openedx-common-settings",
"""FEATURES["ENABLE_DISCUSSION_SERVICE"] = True""",
),
]
)
tutor_hooks.Filters.IMAGES_PUSH.add_item(

# Enable forum v2
tutor_hooks.Filters.CLI_DO_INIT_TASKS.add_item(
(
"forum",
"{{ FORUM_DOCKER_IMAGE }}",
"lms",
# TODO at some point, maybe this flag will be renamed to "forum_v2.enable".
"""
(./manage.py lms waffle_flag --list | grep forum_v2.enable_forum_v2) || ./manage.py lms waffle_flag --create --everyone forum_v2.enable_forum_v2
(./manage.py lms waffle_flag --list | grep forum_v2.enable_mysql_backend) || ./manage.py lms waffle_flag --create --everyone forum_v2.enable_mysql_backend
Comment on lines +47 to +48
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where will the migration of content from mongo to MySQL take place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question :) For now I've left it as a manual step. Basically, people would have to run:

./manage.py lms forum_migrate_course_from_mongodb_to_mysql --no-toggle all
./manage.py lms waffle_flag --create --everyone forum_v2.enable_forum_v2
./manage.py lms forum_delete_course_from_mongodb all

Source: https://github.com/edly-io/forum/?tab=readme-ov-file#migration-from-mongodb-to-mysql

But ideally these steps should be performed automatically as part of tutor local upgrade --from=redwood. But the problem is that currently there exists no filter for that. We should probably create a UPGRADE_TASKS filter.

Comment on lines +47 to +48

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add below to the init task of forum... although migrations will run during launch, just a suggestion.
(./manage.py lms showmigrations forum | grep '\[ \]') && ./manage.py lms migrate forum

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for this condition ./manage.py lms showmigrations forum | grep '\[ \]', it'll throw an error in init jobs if there are no new migrations to apply. The idea was to add ./manage.py lms migrate forum for forum app.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But those migrations will be automatically taken care of during lms init, right? I don't see the need to migrate the forum app individually.

""",
)
)


def get_mongohq_url(
mongo_host: str, mongo_port: str, mongo_database: str, mongo_auth: str
) -> str:
"""
Used to return mongo url, to handle the special case when mongodb+srv used
For more info look at the following PR https://github.com/overhangio/tutor-forum/pull/10
"""
if "mongodb+srv" in mongo_host:
return f"{mongo_host}/{mongo_database}"
return f"mongodb://{mongo_auth}{mongo_host}:{mongo_port}/{mongo_database}"


def get_mongo_auth(monogo_username: str, mongo_password: str) -> str:
if monogo_username and mongo_password:
return f"{monogo_username}:{mongo_password}@"
return ""


tutor_hooks.Filters.ENV_TEMPLATE_VARIABLES.add_items(
[("get_mongohq_url", get_mongohq_url), ("get_mongo_auth", get_mongo_auth)]
)


# Bind-mount repo at runtime
@tutor_hooks.Filters.COMPOSE_MOUNTS.add()
def _mount_cs_comments_service(
volumes: list[tuple[str, str]], name: str
) -> list[tuple[str, str]]:
"""
When mounting cs_comments_service with `--mount=/path/to/cs_comments_service`,
bind-mount the host repo in the forum container.
"""
if name == "cs_comments_service":
repo_path = "/app/cs_comments_service"
volumes += [
("forum", repo_path),
("forum-job", repo_path),
]
return volumes


# Bind-mount repo at build-time
@tutor_hooks.Filters.IMAGES_BUILD_MOUNTS.add()
def _mount_forum_on_build(
mounts: list[tuple[str, str]], host_path: str
) -> list[tuple[str, str]]:
path_basename = os.path.basename(host_path)
if path_basename == "cs_comments_service":
mounts.append(("forum", "forum-src"))
return mounts


# Add the "templates" folder as a template root
tutor_hooks.Filters.ENV_TEMPLATE_ROOTS.add_item(
str(importlib_resources.files("tutorforum") / "templates")
)


def auth_mech_as_ruby(auth_mech: str) -> str:
"""
Convert the authentication mechanism from the format
specified for the Python version to the Ruby version.

https://pymongo.readthedocs.io/en/stable/api/pymongo/database.html#pymongo.auth.MECHANISMS
https://github.com/mongodb/mongo-ruby-driver/blob/932b06b7564a5e5ae8d4ad08fe8d6ceee629e4eb/lib/mongo/auth.rb#L69
"""
return {
"GSSAPI": ":gssapi",
"MONGODB-AWS": ":aws",
"MONGODB-CR": ":mongodb_cr",
"MONGODB-X509": ":mongodb_x509",
"PLAIN": ":plain",
"SCRAM-SHA-1": ":scram",
# SCRAM-256 is only supported from v2.6 of the ruby driver onwards.
# See https://github.com/mongodb/mongo-ruby-driver/releases/tag/v2.6.0
"SCRAM-SHA-256": ":scram",
}.get(auth_mech) or ""


def forum_mongodb_host(host: str) -> str:
"""
Remove the querystring parameters from the mongodb host url. These parameters are
not supported by the outdated mongodb gem. Thus we just trim them out.
"""
# 0 = scheme
# 1 = netloc
# 2 = path
# 3 = params
# 4 = query
# 5 = fragment
parsed = [*urllib.parse.urlparse(host)]
parsed[4] = ""
# We also remove the trailing "/" from the path, which will not play well with the
# full url where we concatenate the database name.
parsed[2] = parsed[2].rstrip("/")
return urllib.parse.urlunparse(parsed)


@FORUM_ENV.add(priority=tutor_hooks.priorities.HIGH)
def _add_base_forum_env(forum_env: dict[str, str]) -> dict[str, str]:
"""
Add environment variables needed for standard build of forum service.
"""
forum_env.update(FORUM_ENV_BASE)
return forum_env


@tutor_hooks.Filters.ENV_PATCHES.add(priority=tutor_hooks.priorities.HIGH)
def _forum_env_patches(patches: list[tuple[str, str]]) -> list[tuple[str, str]]:
"""
Adds environment variables from FORUM_ENV filter to patches.
"""
# The forum service is configured entirely via environment variables. Docker
# Compose and Kubernetes use different syntax to specify environment
# variables. The following code reads environment variables from the
# `FORUM_ENV` filter and rendered in the appropriate format for both so they
# can be included as patches.
k8s_env_patch = ""
local_env_patch = ""
for key, value in FORUM_ENV.apply({}).items():
# Kubernetes
k8s_env_patch += f'- name: {key}\n value: "{value}"\n'
local_env_patch += f'{key}: "{value}"\n'
patches += [("forum-k8s-env", k8s_env_patch), ("forum-local-env", local_env_patch)]
return patches


tutor_hooks.Filters.ENV_TEMPLATE_FILTERS.add_items(
[
("auth_mech_as_ruby", auth_mech_as_ruby),
("forum_mongodb_host", forum_mongodb_host),
]
)
# Render the "build" and "apps" folders
tutor_hooks.Filters.ENV_TEMPLATE_TARGETS.add_items(
[
("forum/build", "plugins"),
("forum/apps", "plugins"),
],
)
# Load patches from files
for path in glob(
os.path.join(
str(importlib_resources.files("tutorforum") / "patches"),
"*",
)
):
with open(path, encoding="utf-8") as patch_file:
tutor_hooks.Filters.ENV_PATCHES.add_item(
(os.path.basename(path), patch_file.read())
)
# Add configuration entries
tutor_hooks.Filters.CONFIG_DEFAULTS.add_items(
[(f"FORUM_{key}", value) for key, value in config.get("defaults", {}).items()]
)
tutor_hooks.Filters.CONFIG_OVERRIDES.add_items(
list(config.get("overrides", {}).items())
)
Loading