feat: allow resource_link change dynamic

Remove the `RICHIE_OPENEDX_SYNC_LOG_REQUESTS` setting, it uses lazy logging. Review documentation fccn/nau-richie-site-factory#198
fccn · May 29, 2024 · a0462d9 · a0462d9
1 parent 8315440
commit a0462d9
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 79 deletions.
diff --git a/docs/configuration.md b/docs/configuration.md
@@ -0,0 +1,47 @@
+# Configuration
+
+## Settings
+
+- `INSTALLED_APPS` need to include `richie_openedx_sync` to install this application
+- `RICHIE_OPENEDX_SYNC_COURSE_HOOKS` the most important configuration. Could be configured globally on using Django settings or per organization using multi-site site configuration. This hooks consists of a list of configurations. It is required the `secret` and `url`, the other are optional - `timeout` and `resource_link_template`.
+
+Python:
+```python
+RICHIE_OPENEDX_SYNC_COURSE_HOOKS=[
+    {
+        "secret": "changeme",
+        "url": "http://richie.local.dev:8070/api/v1.0/course-runs-sync/",
+        "timeout": "6",
+        "resource_link_template": "http://{lms_domain}/courses/{course_id}/info",
+    },
+]
+```
+
+JSON on site configuration:
+```json
+"RICHIE_OPENEDX_SYNC_COURSE_HOOKS": [
+    {
+        "secret": "changeme",
+        "url": "http://richie.local.dev:8070/api/v1.0/course-runs-sync/",
+        "timeout": "6",
+        "resource_link_template": "http://{lms_domain}/courses/{course_id}/info"
+    }
+]
+```
+
+## Multi site
+If you have a multi site instance, you can configure a specific hook for that Organization.
+
+Example of Open edX [Site Configuration](http://localhost:18000/admin/site_configuration/siteconfiguration/1/change/)
+Django administration page add the next configurations:
+
+```json
+"RICHIE_OPENEDX_SYNC_COURSE_HOOKS": [
+    {
+        "secret": "changeme",
+        "url": "http://richie.local.dev:8070/api/v1.0/course-runs-sync/",
+        "timeout": "6",
+        "resource_link_template": "http://{lms_domain}/courses/{course_id}/info"
+    }
+]
+```
diff --git a/docs/installation_devstack.md b/docs/installation_devstack.md
@@ -33,9 +33,25 @@ So you should have your folders like this on your machine:
 ```
 
 ## Active the `richie_openedx_sync` Django application
-On the Open edX devstack edit the file and add the `richie_openedx_sync` app
-to the `INSTALLED_APPS` on the file `cms/envs/devstack.py` and `lms/envs/devstack.py` 
-within edx-platform.
+On the Open edX devstack, inside the `edx-plaform` project edit the files:
+- `cms/envs/private.py`
+- `lms/envs/private.py`
+With the content of:
+```python
+from .devstack import INSTALLED_APPS
+from .devstack import INSTALLED_APPS
+INSTALLED_APPS += ['richie_openedx_sync']
+RICHIE_OPENEDX_SYNC_COURSE_HOOKS=[
+    {
+        "secret": "changeme",
+        "url": "http://richie.local.dev:8070/api/v1.0/course-runs-sync/",
+        "timeout": "6",
+        "resource_link_template": "http://{lms_domain}/courses/{course_id}/info",
+    },
+]
+```
+This adds the `richie_openedx_sync` application to the Django installed applications,
+increase the log verbosity and configure a globally hook for all organizations courses.
 
 ## Hosts
 When your are developing the connection between Open edX and Richie, because both stacks aren't
@@ -49,23 +65,24 @@ Find your local IP Address, eg. like 192.168....
 
 Add the next line to the `/etc/hosts` file of your host machine:
 ```bash
-make studio-shell
+make dev.shell.studio
 vim /etc/hosts
 <Your local IP Address> richie.local.dev
 ```
 
-## Open edX site configuration
+## Multi site
+If you have a multi site instance, you can configure a specific hook for that Organization.
 
-Open your Open edX devstack, eg. http://localhost:18000, on the 
-[Site Configuration](http://localhost:18000/admin/site_configuration/siteconfiguration/1/change/)
-Django administration page add the next configurations ( don't forget to replace the secret! ):
+Example of Open edX [Site Configuration](http://localhost:18000/admin/site_configuration/siteconfiguration/1/change/)
+Django administration page add the next configurations:
 
 ```json
 "RICHIE_OPENEDX_SYNC_COURSE_HOOKS": [
     {
         "secret": "changeme",
         "url": "http://richie.local.dev:8070/api/v1.0/course-runs-sync/",
-        "timeout": "6"
+        "timeout": "6",
+        "resource_link_template": "http://{lms_domain}/courses/{course_id}/info"
     }
 ]
 ```

diff --git a/richie_openedx_sync/settings.py b/richie_openedx_sync/settings.py
@@ -1,17 +1,7 @@
 from django.conf import settings
 
 # Load `RICHIE_OPENEDX_SYNC_COURSE_HOOKS` setting using the open edX `ENV_TOKENS` production mode.
-# This requires the `RICHIE_OPENEDX_SYNC_COURSE_HOOKS` should be added to the `EDXAPP_ENV_EXTRA`
-# ansible deployment configuration.
 settings.RICHIE_OPENEDX_SYNC_COURSE_HOOKS = getattr(settings, "ENV_TOKENS", {}).get(
     "RICHIE_OPENEDX_SYNC_COURSE_HOOKS",
-    getattr(settings, "RICHIE_OPENEDX_SYNC_COURSE_HOOKS", None),
-)
-
-# Load `RICHIE_OPENEDX_SYNC_LOG_REQUESTS` setting using the open edX `ENV_TOKENS` production mode.
-# This requires the `RICHIE_OPENEDX_SYNC_LOG_REQUESTS` should be added to the `EDXAPP_ENV_EXTRA`
-# ansible deployment configuration.
-settings.RICHIE_OPENEDX_SYNC_LOG_REQUESTS = getattr(settings, "ENV_TOKENS", {}).get(
-    "RICHIE_OPENEDX_SYNC_LOG_REQUESTS",
-    getattr(settings, "RICHIE_OPENEDX_SYNC_LOG_REQUESTS", False),
+    getattr(settings, "RICHIE_OPENEDX_SYNC_COURSE_HOOKS", []),
 )
diff --git a/richie_openedx_sync/tasks.py b/richie_openedx_sync/tasks.py
@@ -38,6 +38,17 @@ def sync_course_run_information_to_richie(*args, **kwargs) -> Dict[str, bool]:
         raise ValueError("No course found with the course_id '{}'".format(course_id))
 
     org = course_key.org
+
+    hooks = configuration_helpers.get_value_for_org(
+        org,
+        "RICHIE_OPENEDX_SYNC_COURSE_HOOKS",
+        getattr(settings, "RICHIE_OPENEDX_SYNC_COURSE_HOOKS"),
+    )
+    if len(hooks) == 0:
+        log.info("No richie course hook found for organization '%s'. Please configure the "
+            "'RICHIE_OPENEDX_SYNC_COURSE_HOOKS' setting or as site configuration", org)
+        return {}
+
     lms_domain = configuration_helpers.get_value_for_org(
         org, "LMS_BASE", settings.LMS_BASE
     )
@@ -50,92 +61,65 @@ def sync_course_run_information_to_richie(*args, **kwargs) -> Dict[str, bool]:
     # course start date for the enrollment start date when the enrollment start date isn't defined.
     enrollment_start = enrollment_start or course_start
 
-    resource_link = configuration_helpers.get_value_for_org(
-        org,
-        "RICHIE_OPENEDX_SYNC_RESOURCE_LINK",
-        getattr(
-            settings,
-            "RICHIE_OPENEDX_SYNC_RESOURCE_LINK",
-            "https://{lms_domain}/courses/{course_id}/info",
-        ),
-    ).format(lms_domain=lms_domain, course_id=str(course_id))
-
-    enrollment_count = CourseEnrollment.objects.filter(course_id=course_id).count()
-
-    data = {
-        "resource_link": resource_link,
-        "start": course_start,
-        "end": course_end,
-        "enrollment_start": enrollment_start,
-        "enrollment_end": enrollment_end,
-        "languages": [course.language or settings.LANGUAGE_CODE],
-        "enrollment_count": enrollment_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", []),
-    )
-    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 {}
-
-    log_requests = configuration_helpers.get_value_for_org(
-        org,
-        "RICHIE_OPENEDX_SYNC_LOG_REQUESTS",
-        getattr(settings, "RICHIE_OPENEDX_SYNC_LOG_REQUESTS", False),
-    )
+    enrollment_count = None
 
     result = {}
 
     for hook in hooks:
+        # calculate enrollment count just once per hook
+        if not enrollment_count:
+            enrollment_count = CourseEnrollment.objects.filter(
+                course_id=course_id
+            ).count()
+
+        resource_link = hook.get(
+            "resource_link_template", "https://{lms_domain}/courses/{course_id}/info"
+        ).format(lms_domain=lms_domain, course_id=str(course_id))
+
+        data = {
+            "resource_link": resource_link,
+            "start": course_start,
+            "end": course_end,
+            "enrollment_start": enrollment_start,
+            "enrollment_end": enrollment_end,
+            "languages": [course.language or settings.LANGUAGE_CODE],
+            "enrollment_count": enrollment_count,
+            "catalog_visibility": course.catalog_visibility,
+        }
+
         signature = hmac.new(
             hook["secret"].encode("utf-8"),
             msg=json.dumps(data).encode("utf-8"),
             digestmod=hashlib.sha256,
         ).hexdigest()
 
-        richie_url = hook.get("url")
+        richie_url = str(hook.get("url"))
         timeout = int(hook.get("timeout", 20))
 
         try:
+            log.info("Sending to Richie %s the data %s", richie_url, str(data))
             response = requests.post(
                 richie_url,
                 json=data,
-                headers={"Authorization": "SIG-HMAC-SHA256 {:s}".format(signature)},
+                headers={"Authorization": "SIG-HMAC-SHA256 {signature}".format(signature=signature)},
                 timeout=timeout,
             )
             response.raise_for_status()
             result[richie_url] = True
-            if log_requests:
-                status_code = response.status_code
-                msg = "Synchronized the course {} to richie site {} it returned the HTTP status code {}".format(
-                    course_key, richie_url, status_code
-                )
-                log.info(msg)
-                log.info(response.content)
+
+            log.info("Synchronized the course %s to richie site %s it returned the HTTP status code %d response content: %s".format(
+                course_id, richie_url, response.status_code, response.content
+            ))
         except requests.exceptions.HTTPError as e:
-            status_code = response.status_code
-            msg = "Error synchronizing course {} to richie site {} it returned the HTTP status code {}".format(
-                course_key, richie_url, status_code
+            log.warning("Error synchronizing course %s to richie site %s it returned the HTTP status code %d with response content of %s",
+                course_id, richie_url, response.status_code, response.content
             )
             log.warning(e, exc_info=True)
-            log.warning(msg)
-            log.warning(response.content)
             result[richie_url] = False
 
         except requests.exceptions.RequestException as e:
-            msg = "Error synchronizing course {} to richie site {}".format(
-                course_key, richie_url
-            )
+            log.warning("Error synchronizing course %s to richie site %s", course_id, richie_url)
             log.warning(e, exc_info=True)
-            log.warning(msg)
             result[richie_url] = False
 
     return result