From 4d979dd05998186e3e04e42eaa5d7b673582b97f Mon Sep 17 00:00:00 2001 From: Ivo Branco Date: Thu, 20 Jan 2022 17:34:35 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20multiple=20richie=20sites:=20err?= =?UTF-8?q?or=20on=201st,=202nd=20is=20not=20called?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When configured with multiple Richie sites, don't stop on the 1st error. Closes #5 --- CHANGELOG.md | 4 + .../commands/sync_courses_to_richie.py | 100 +++++++++++------- richie_openedx_sync/tasks.py | 58 ++++++---- 3 files changed, 101 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 245bc10..34e9e1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## Fixed + +- When configured with multiple Richie sites, don't stop on the 1st error. + ## Added - Send course `catalog_visibility` to richie diff --git a/richie_openedx_sync/management/commands/sync_courses_to_richie.py b/richie_openedx_sync/management/commands/sync_courses_to_richie.py index 831c113..f10da37 100644 --- a/richie_openedx_sync/management/commands/sync_courses_to_richie.py +++ b/richie_openedx_sync/management/commands/sync_courses_to_richie.py @@ -1,8 +1,10 @@ """ -Script for synchronize courses to Richie marketing site +Script to synchronize courses to Richie marketing site """ +import logging +from typing import Dict from django.core.management.base import BaseCommand from six import text_type @@ -10,61 +12,77 @@ from richie_openedx_sync.tasks import sync_course_run_information_to_richie from opaque_keys.edx.keys import CourseKey +log = logging.getLogger(__name__) + class Command(BaseCommand): """ - Synchronize courses from mongo to the Richie marketing site + Command that synchronizes the Open edX courses to the Richie marketing site """ + help = ( - 'Synchronize courses from mongo to the Richie marketing site, all courses ' - 'or a specific course' + "Synchronize courses to the Richie marketing site, by default all courses " + "or a specific course" ) def add_arguments(self, parser): - parser.add_argument('--course_id', type=str, default=None, - help='Course id to synchronize, otherwise all courses would be sync') + parser.add_argument( + "--course_id", + type=str, + default=None, + help="Course id to synchronize, otherwise all courses would be sync", + ) def handle(self, *args, **kwargs): """ - Execute the command + Synchronize courses to the Richie marketing site, print to console its sync progress. """ - course_id = kwargs['course_id'] - courses, failed_courses = synchronize_courses(course_id=course_id) + course_id = kwargs["course_id"] - print("=" * 80) - print("=" * 30 + "> Synchronization summary") - print(u"Total number of courses to sync: {0}".format(len(courses))) - print(u"Total number of courses which failed to sync: {0}".format(len(failed_courses))) - print("List of sync failed courses ids:") - print("\n".join(failed_courses)) - print("=" * 80) + if not course_id: + module_store = modulestore() + courses = module_store.get_courses() + course_ids = [x.id for x in courses] + else: + course_key = CourseKey.from_string(course_id) + course = modulestore().get_course(course_key) + courses = [course] + course_ids = [course_id] + course_ids_count = len(course_ids) + total_sync_ok_count = 0 + total_sync_not_ok_count = 0 -def synchronize_courses(course_id=None): - """ - Export all courses to target directory and return the list of courses which failed to export - """ - if not course_id: - module_store = modulestore() - courses = module_store.get_courses() - course_ids = [x.id for x in courses] - else: - course_key = CourseKey.from_string(course_id) - course = modulestore().get_course(course_key) - courses = [course] - course_ids = [course_id] + for course_id in course_ids: + log.info("-" * 80) + log.info("Synchronizing to Richie course id = {0}".format(course_id)) + sync_result = sync_course_run_information_to_richie( + course_id=str(course_id) + ) + ok_count = len(list(filter(lambda e: e[1], sync_result.items()))) + not_ok_count = len(list(filter(lambda e: not e[1], sync_result.items()))) + if ok_count > 0: + log.info(" ok count: {0}".format(ok_count)) + if not_ok_count > 0: + log.info(" not ok count: {0}".format(not_ok_count)) - failed_courses = [] + richie_failed_backends = str( + list( + map( + lambda e: e[0], + (filter(lambda e: not e[1], sync_result.items())), + ) + ) + ) + log.info(" failed backends: {0}".format(richie_failed_backends)) - for course_id in course_ids: - print("-" * 80) - print(u"Synchronizing to Richie course id = {0}".format(course_id)) - try: - sync_course_run_information_to_richie(course_id=str(course_id)) - except Exception as err: # pylint: disable=broad-except - failed_courses.append(text_type(course_id)) - print(u"=" * 30 + u"> Oops, failed to sync {0}".format(course_id)) - print("Error:") - print(err) + total_sync_ok_count += ok_count + total_sync_not_ok_count += not_ok_count - return courses, failed_courses + log.info("=" * 80) + log.info("Synchronization summary") + print("Total number of courses synchronized: {0}".format(course_ids_count)) + log.info("Total number of synchronizations ok: {0}".format(total_sync_ok_count)) + log.info( + "Total number of synchronizations not ok (error): {0}".format(total_sync_not_ok_count) + ) diff --git a/richie_openedx_sync/tasks.py b/richie_openedx_sync/tasks.py index a87f70d..09764df 100644 --- a/richie_openedx_sync/tasks.py +++ b/richie_openedx_sync/tasks.py @@ -1,26 +1,34 @@ import hashlib import hmac import json -import requests import logging +from typing import Dict +import requests from celery import shared_task from django.conf import settings -from xmodule.modulestore.django import modulestore -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from student.models import CourseEnrollment - +from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) @shared_task -def sync_course_run_information_to_richie(*args, **kwargs): +def sync_course_run_information_to_richie(*args, **kwargs) -> Dict[str, bool]: """ Synchronize an OpenEdX course run, identified by its course key, to all Richie instances. + + Raises: + ValueError: when course if not found + + Returns: + dict: where the key is the richie url and the value is a boolean if the synchronization + was ok. """ - log.info("Entering richie update course on publish") + + log.debug("Entering richie update course on publish") course_id = kwargs["course_id"] course_key = CourseKey.from_string(course_id) @@ -31,9 +39,7 @@ def sync_course_run_information_to_richie(*args, **kwargs): org = course_key.org edxapp_domain = configuration_helpers.get_value_for_org( - org, - "LMS_BASE", - settings.LMS_BASE + org, "LMS_BASE", settings.LMS_BASE ) data = { @@ -42,25 +48,30 @@ def sync_course_run_information_to_richie(*args, **kwargs): ), "start": course.start and course.start.isoformat(), "end": course.end and course.end.isoformat(), - "enrollment_start": course.enrollment_start and course.enrollment_start.isoformat(), + "enrollment_start": course.enrollment_start + and course.enrollment_start.isoformat(), "enrollment_end": course.enrollment_end and course.enrollment_end.isoformat(), "languages": [course.language or settings.LANGUAGE_CODE], - "enrollment_count": CourseEnrollment.objects.filter(course_id=course_id).count(), + "enrollment_count": CourseEnrollment.objects.filter( + course_id=course_id + ).count(), "catalog_visibility": course.catalog_visibility, } hooks = configuration_helpers.get_value_for_org( org, - 'RICHIE_OPENEDX_SYNC_COURSE_HOOKS', - getattr(settings, "RICHIE_OPENEDX_SYNC_COURSE_HOOKS", []) + "RICHIE_OPENEDX_SYNC_COURSE_HOOKS", + getattr(settings, "RICHIE_OPENEDX_SYNC_COURSE_HOOKS", []), ) - if len(hooks) == 0: + if not hooks: msg = ( "No richie course hook found for organization '{}'. Please configure the " "'RICHIE_OPENEDX_SYNC_COURSE_HOOKS' setting or as site configuration" ).format(org) log.info(msg) - return msg + return {} + + result = {} for hook in hooks: signature = hmac.new( @@ -71,7 +82,7 @@ def sync_course_run_information_to_richie(*args, **kwargs): richie_url = hook.get("url") timeout = int(hook.get("timeout", 5)) - + try: response = requests.post( richie_url, @@ -80,15 +91,22 @@ def sync_course_run_information_to_richie(*args, **kwargs): timeout=timeout, ) response.raise_for_status() + result[richie_url] = True except requests.exceptions.HTTPError as e: status_code = response.status_code log.debug(e, exc_info=True) - msg = "Error synchronizing course {} to richie site {} it returned the HTTP status code {}".format(course_key, richie_url, status_code) + msg = "Error synchronizing course {} to richie site {} it returned the HTTP status code {}".format( + course_key, richie_url, status_code + ) log.error(msg) - raise e + result[richie_url] = False + except requests.exceptions.RequestException as e: log.debug(e, exc_info=True) - msg = "Error synchronizing course {} to richie site {}".format(course_key, richie_url) + msg = "Error synchronizing course {} to richie site {}".format( + course_key, richie_url + ) log.error(msg) - raise e + result[richie_url] = False + return result