From a9739ba638738cd9a9a5c5ae244525fae9497262 Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Thu, 12 Sep 2024 14:25:11 +0500 Subject: [PATCH] feat: add a method for calling python APIs - add a method named `perform_v2_request` to call python method instead of calling HTTPs APIs - Use `perform_v2_request` method for pin, unpin thread and get user's data by user_id --- .../comment_client/thread.py | 29 +++---- .../comment_client/user.py | 14 ++-- .../comment_client/utils.py | 80 ++++++++++++++++++- 3 files changed, 94 insertions(+), 29 deletions(-) diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/thread.py b/openedx/core/djangoapps/django_comment_common/comment_client/thread.py index ef5accbad25..19edd14f723 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/thread.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/thread.py @@ -6,6 +6,9 @@ from eventtracking import tracker from . import models, settings, utils +from forum import api as forum_api +from utils import check_service_enabled, handle_metric_tags, handle_response + log = logging.getLogger(__name__) @@ -193,27 +196,17 @@ def unFlagAbuse(self, user, voteable, removeAll): voteable._update_from_response(response) def pin(self, user, thread_id): - url = _url_for_pin_thread(thread_id) - params = {'user_id': user.id} - response = utils.perform_request( - 'put', - url, - params, - metric_tags=self._metric_tags, - metric_action='thread.pin' - ) + check_service_enabled() + response = forum_api.pin_thread(user.id, thread_id) + handle_metric_tags("put", response.status_code, metric_action="thread.pin", metric_tags=self._metric_tags,) + response = handle_response(response, "put", {'user_id': user.id}, metric_action="thread.pin",) self._update_from_response(response) def un_pin(self, user, thread_id): - url = _url_for_un_pin_thread(thread_id) - params = {'user_id': user.id} - response = utils.perform_request( - 'put', - url, - params, - metric_tags=self._metric_tags, - metric_action='thread.unpin' - ) + check_service_enabled() + response = forum_api.unpin_thread(user.id, thread_id) + handle_metric_tags("put", response.status_code, metric_action="thread.unpin", metric_tags=self._metric_tags,) + response = handle_response(response, "put", {'user_id': user.id}, metric_action="thread.unpin",) self._update_from_response(response) diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/user.py b/openedx/core/djangoapps/django_comment_common/comment_client/user.py index 684469c9e78..7c3a7bab131 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/user.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/user.py @@ -3,7 +3,8 @@ from . import models, settings, utils - +from forum import api as forum_api +from utils import check_service_enabled, handle_metric_tags, handle_response class User(models.Model): @@ -149,13 +150,10 @@ def _retrieve(self, *args, **kwargs): if self.attributes.get('group_id'): retrieve_params['group_id'] = self.group_id try: - response = utils.perform_request( - 'get', - url, - retrieve_params, - metric_action='model.retrieve', - metric_tags=self._metric_tags, - ) + check_service_enabled() + response = forum_api.retrieve_user(retrieve_params, user_id=self.attributes["id"]) + handle_metric_tags("get", response.status_code, metric_action="model.retrieve", metric_tags=self._metric_tags) + response = handle_response(response, "get", retrieve_params, metric_action="model.retrieve") except utils.CommentClientRequestError as e: if e.status_code == 404: # attempt to gracefully recover from a previous failure diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/utils.py b/openedx/core/djangoapps/django_comment_common/comment_client/utils.py index b47f12c6c22..0467bdb0375 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/utils.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/utils.py @@ -76,6 +76,9 @@ def perform_request(method, url, data_or_params=None, raw=False, ) # For the better logging + response_to_log_and_compare = ( + response.json() if response.content else response.content + ) log.info( """ ======> FORUM <====== @@ -87,7 +90,13 @@ def perform_request(method, url, data_or_params=None, raw=False, response: {response} ======> END <====== - """.format(method=method, url=url, params=params, data=data, response=response.json()) + """.format( + method=method, + url=url, + params=params, + data=data, + response=response_to_log_and_compare, + ) ) if method == "get": @@ -100,12 +109,17 @@ def perform_request(method, url, data_or_params=None, raw=False, headers=headers, timeout=config.connection_timeout, ) + forum_v1_response_to_log_and_compare = ( + forum_v1_response.json() + if forum_v1_response.content + else forum_v1_response.content + ) log.info(f"requested forum proxey url: {url}") log.info(f"requested forum v1 url: {forum_v1_url}") - if forum_v1_response.json() != response.json(): + if forum_v1_response_to_log_and_compare != response_to_log_and_compare: log.error( f"Forum v2 difference, for endpoint {forum_v1_url} with params={params}. \ - Expected: {forum_v1_response.json()}. Got: {response.json()}." + Expected: {forum_v1_response_to_log_and_compare}. Got: {response_to_log_and_compare}." ) metric_tags.append(f'status_code:{response.status_code}') @@ -204,3 +218,63 @@ def check_forum_heartbeat(): return 'forum', False, res.get('check', 'Forum heartbeat failed') except Exception as fail: return 'forum', False, str(fail) + + +def check_service_enabled(): + # To avoid dependency conflict + from openedx.core.djangoapps.django_comment_common.models import ForumsConfig + + config = ForumsConfig.current() + + if not config.enabled: + raise CommentClientMaintenanceError("service disabled") + + +def handle_metric_tags( + request_method, status_code, metric_action=None, metric_tags=None +): + if metric_tags is None: + metric_tags = [] + + metric_tags.append(f"method:{request_method}") + if metric_action: + metric_tags.append(f"action:{metric_action}") + + metric_tags.append(f"status_code:{status_code}") + if status_code > 200: + metric_tags.append("result:failure") + else: + metric_tags.append("result:success") + + +def handle_response( + response, + request_method, + params, + raw=False, + metric_action=None, +): + status_code = response.status_code + if 200 < status_code < 500: # lint-amnesty, pylint: disable=no-else-raise + log.info( + f"Investigation Log: CommentClientRequestError for request with {request_method} and params {params}" + ) + raise CommentClientRequestError(response["text"], status_code) + # Heroku returns a 503 when an application is in maintenance mode + elif status_code == 503: + raise CommentClientMaintenanceError(response["text"]) + elif status_code == 500: + raise CommentClient500Error(response["text"]) + else: + if raw: + return response.text + else: + try: + data = response["data"] + except ValueError: + raise CommentClientError( # lint-amnesty, pylint: disable=raise-missing-from + "Invalid JSON response for request to forum v2 method named{forum_v2_method}; first 100 characters: '{content}'".format( + forum_v2_method=metric_action, content=response.text[:100] + ) + ) + return data