From 0a83bee18341452cc26aea8020be7d1a8ec1f04c Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Thu, 1 Aug 2024 16:24:52 +0000 Subject: [PATCH] feat: removing the from the retirement pipeline although the demographics IDA has not been entirely deprecated as yet, it is about to be, so removing from the retirement pipeline. --- .../tests/utils/test_edx_api.py | 42 +--- scripts/user_retirement/utils/edx_api.py | 212 ++++++++---------- 2 files changed, 92 insertions(+), 162 deletions(-) diff --git a/scripts/user_retirement/tests/utils/test_edx_api.py b/scripts/user_retirement/tests/utils/test_edx_api.py index 706b1ce6d4a7..eb826cd1a4fd 100644 --- a/scripts/user_retirement/tests/utils/test_edx_api.py +++ b/scripts/user_retirement/tests/utils/test_edx_api.py @@ -21,7 +21,7 @@ FAKE_USERNAMES, TEST_RETIREMENT_QUEUE_STATES, TEST_RETIREMENT_STATE, - get_fake_user_retirement + get_fake_user_retirement, ) from scripts.user_retirement.utils import edx_api @@ -499,46 +499,6 @@ def test_replace_usernames(self, mock_method): ) -class TestDemographicsApi(OAuth2Mixin, unittest.TestCase): - """ - Test the edX Demographics API client. - """ - - @responses.activate(registry=OrderedRegistry) - def setUp(self): - super().setUp() - self.mock_access_token_response() - self.lms_base_url = 'http://localhost:18000/' - self.demographics_base_url = 'http://localhost:18360/' - self.demographics_api = edx_api.DemographicsApi( - self.lms_base_url, - self.demographics_base_url, - 'the_client_id', - 'the_client_secret' - ) - - def tearDown(self): - super().tearDown() - responses.reset() - - @patch.object(edx_api.DemographicsApi, 'retire_learner') - def test_retire_learner(self, mock_method): - json_data = { - 'lms_user_id': get_fake_user_retirement()['user']['id'] - } - responses.add( - POST, - urljoin(self.demographics_base_url, 'demographics/api/v1/retire_demographics/'), - match=[matchers.json_params_matcher(json_data)] - ) - self.demographics_api.retire_learner( - learner=get_fake_user_retirement() - ) - mock_method.assert_called_once_with( - learner=get_fake_user_retirement() - ) - - class TestLicenseManagerApi(OAuth2Mixin, unittest.TestCase): """ Test the edX License Manager API client. diff --git a/scripts/user_retirement/utils/edx_api.py b/scripts/user_retirement/utils/edx_api.py index 10903640456a..e891f04019a9 100644 --- a/scripts/user_retirement/utils/edx_api.py +++ b/scripts/user_retirement/utils/edx_api.py @@ -1,6 +1,7 @@ """ edX API classes which call edX service REST API endpoints using the edx-rest-api-client module. """ + import logging from urllib.parse import urljoin @@ -28,6 +29,7 @@ class BaseApiClient: """ API client base class used to submit API requests to a particular web service. """ + append_slash = True _access_token = None @@ -45,15 +47,15 @@ def get_api_url(self, path): Args: path (str): API endpoint path. """ - path = path.strip('/') + path = path.strip("/") if self.append_slash: - path += '/' + path += "/" - return urljoin(f'{self.api_base_url}/', path) + return urljoin(f"{self.api_base_url}/", path) def _request(self, method, url, log_404_as_error=True, **kwargs): - if 'headers' not in kwargs: - kwargs['headers'] = {'Content-type': 'application/json'} + if "headers" not in kwargs: + kwargs["headers"] = {"Content-type": "application/json"} try: response = requests.request(method, url, auth=SuppliedJwtAuth(self._access_token), **kwargs) @@ -68,7 +70,7 @@ def _request(self, method, url, log_404_as_error=True, **kwargs): # Immediately raise the error so that a 404 isn't logged as an API error in this case. raise HttpDoesNotExistException(str(exc)) - LOG.error(f'API Error: {str(exc)} with status code: {status_code}') + LOG.error(f"API Error: {str(exc)} with status code: {status_code}") if status_code == 504: # Differentiate gateway errors so different backoff can be used. @@ -92,31 +94,31 @@ def get_access_token(oauth_base_url, client_id, client_secret): Returns: str: JWT access token """ - oauth_access_token_url = urljoin(f'{oauth_base_url}/', OAUTH_ACCESS_TOKEN_URL) + oauth_access_token_url = urljoin(f"{oauth_base_url}/", OAUTH_ACCESS_TOKEN_URL) data = { - 'grant_type': 'client_credentials', - 'client_id': client_id, - 'client_secret': client_secret, - 'token_type': 'jwt', + "grant_type": "client_credentials", + "client_id": client_id, + "client_secret": client_secret, + "token_type": "jwt", } try: response = requests.post( oauth_access_token_url, data=data, headers={ - 'User-Agent': 'scripts.user_retirement', + "User-Agent": "scripts.user_retirement", }, - timeout=(REQUEST_CONNECT_TIMEOUT, REQUEST_READ_TIMEOUT) + timeout=(REQUEST_CONNECT_TIMEOUT, REQUEST_READ_TIMEOUT), ) response.raise_for_status() - return response.json()['access_token'] + return response.json()["access_token"] except KeyError as exc: - LOG.error(f'Failed to get token. {str(exc)} does not exist.') + LOG.error(f"Failed to get token. {str(exc)} does not exist.") raise except HTTPError as exc: LOG.error( - f'API Error: {str(exc)} with status code: {exc.response.status_code} fetching access token: {client_id}' + f"API Error: {str(exc)} with status code: {exc.response.status_code} fetching access token: {client_id}" ) raise @@ -125,7 +127,7 @@ def _backoff_handler(details): """ Simple logging handler for when timeout backoff occurs. """ - LOG.info('Trying again in {wait:0.1f} seconds after {tries} tries calling {target}'.format(**details)) + LOG.info("Trying again in {wait:0.1f} seconds after {tries} tries calling {target}".format(**details)) def _wait_one_minute(): @@ -143,10 +145,7 @@ def _giveup_on_unexpected_exception(exc): # Treat a ConnectionError as retryable. isinstance(exc, ConnectionError) # All 5xx status codes are retryable except for 504 Gateway Timeout. - or ( - 500 <= exc.response.status_code < 600 - and exc.response.status_code != 504 # Gateway Timeout - ) + or (500 <= exc.response.status_code < 600 and exc.response.status_code != 504) # Gateway Timeout # Status code 104 is unreserved, but we must have added this because we observed retryable 104 responses. or exc.response.status_code == 104 ) @@ -167,7 +166,7 @@ def inner(func): # pylint: disable=missing-docstring # Wrap the actual _backoff_handler so that we can patch the real one in unit tests. Otherwise, the func # will get decorated on import, embedding this handler as a python object reference, precluding our ability # to patch it in tests. - on_backoff=lambda details: _backoff_handler(details) # pylint: disable=unnecessary-lambda + on_backoff=lambda details: _backoff_handler(details), # pylint: disable=unnecessary-lambda ) func_with_timeout_backoff = backoff.on_exception( _wait_one_minute, @@ -176,7 +175,7 @@ def inner(func): # pylint: disable=missing-docstring # Wrap the actual _backoff_handler so that we can patch the real one in unit tests. Otherwise, the func # will get decorated on import, embedding this handler as a python object reference, precluding our ability # to patch it in tests. - on_backoff=lambda details: _backoff_handler(details) # pylint: disable=unnecessary-lambda + on_backoff=lambda details: _backoff_handler(details), # pylint: disable=unnecessary-lambda ) return func_with_backoff(func_with_timeout_backoff(func)) @@ -193,14 +192,11 @@ def learners_to_retire(self, states_to_request, cool_off_days=7, limit=None): """ Retrieves a list of learners awaiting retirement actions. """ - params = { - 'cool_off_days': cool_off_days, - 'states': states_to_request - } + params = {"cool_off_days": cool_off_days, "states": states_to_request} if limit: - params['limit'] = limit - api_url = self.get_api_url('api/user/v1/accounts/retirement_queue') - return self._request('GET', api_url, params=params) + params["limit"] = limit + api_url = self.get_api_url("api/user/v1/accounts/retirement_queue") + return self._request("GET", api_url, params=params) @_retry_lms_api() def get_learners_by_date_and_status(self, state_to_request, start_date, end_date): @@ -214,20 +210,20 @@ def get_learners_by_date_and_status(self, state_to_request, start_date, end_date :param end_date: Date or Datetime """ params = { - 'start_date': start_date.strftime('%Y-%m-%d'), - 'end_date': end_date.strftime('%Y-%m-%d'), - 'state': state_to_request + "start_date": start_date.strftime("%Y-%m-%d"), + "end_date": end_date.strftime("%Y-%m-%d"), + "state": state_to_request, } - api_url = self.get_api_url('api/user/v1/accounts/retirements_by_status_and_date') - return self._request('GET', api_url, params=params) + api_url = self.get_api_url("api/user/v1/accounts/retirements_by_status_and_date") + return self._request("GET", api_url, params=params) @_retry_lms_api() def get_learner_retirement_state(self, username): """ Retrieves the given learner's retirement state. """ - api_url = self.get_api_url(f'api/user/v1/accounts/{username}/retirement_status') - return self._request('GET', api_url) + api_url = self.get_api_url(f"api/user/v1/accounts/{username}/retirement_status") + return self._request("GET", api_url) @_retry_lms_api() def update_learner_retirement_state(self, username, new_state_name, message, force=False): @@ -235,26 +231,22 @@ def update_learner_retirement_state(self, username, new_state_name, message, for Updates the given learner's retirement state to the retirement state name new_string with the additional string information in message (for logging purposes). """ - data = { - 'username': username, - 'new_state': new_state_name, - 'response': message - } + data = {"username": username, "new_state": new_state_name, "response": message} if force: - data['force'] = True + data["force"] = True - api_url = self.get_api_url('api/user/v1/accounts/update_retirement_status') - return self._request('PATCH', api_url, json=data) + api_url = self.get_api_url("api/user/v1/accounts/update_retirement_status") + return self._request("PATCH", api_url, json=data) @_retry_lms_api() def retirement_deactivate_logout(self, learner): """ Performs the user deactivation and forced logout step of learner retirement """ - data = {'username': learner['original_username']} - api_url = self.get_api_url('api/user/v1/accounts/deactivate_logout') - return self._request('POST', api_url, json=data) + data = {"username": learner["original_username"]} + api_url = self.get_api_url("api/user/v1/accounts/deactivate_logout") + return self._request("POST", api_url, json=data) @_retry_lms_api() def retirement_retire_forum(self, learner): @@ -262,10 +254,10 @@ def retirement_retire_forum(self, learner): Performs the forum retirement step of learner retirement """ # api/discussion/ - data = {'username': learner['original_username']} + data = {"username": learner["original_username"]} try: - api_url = self.get_api_url('api/discussion/v1/accounts/retire_forum') - return self._request('POST', api_url, json=data) + api_url = self.get_api_url("api/discussion/v1/accounts/retire_forum") + return self._request("POST", api_url, json=data) except HttpDoesNotExistException: LOG.info("No information about learner retirement") return True @@ -275,18 +267,18 @@ def retirement_retire_mailings(self, learner): """ Performs the email list retirement step of learner retirement """ - data = {'username': learner['original_username']} - api_url = self.get_api_url('api/user/v1/accounts/retire_mailings') - return self._request('POST', api_url, json=data) + data = {"username": learner["original_username"]} + api_url = self.get_api_url("api/user/v1/accounts/retire_mailings") + return self._request("POST", api_url, json=data) @_retry_lms_api() def retirement_unenroll(self, learner): """ Unenrolls the user from all courses """ - data = {'username': learner['original_username']} - api_url = self.get_api_url('api/enrollment/v1/unenroll') - return self._request('POST', api_url, json=data) + data = {"username": learner["original_username"]} + api_url = self.get_api_url("api/enrollment/v1/unenroll") + return self._request("POST", api_url, json=data) # This endpoint additionally returns 500 when the EdxNotes backend service is unavailable. @_retry_lms_api() @@ -294,9 +286,9 @@ def retirement_retire_notes(self, learner): """ Deletes all the user's notes (aka. annotations) """ - data = {'username': learner['original_username']} - api_url = self.get_api_url('api/edxnotes/v1/retire_user') - return self._request('POST', api_url, json=data) + data = {"username": learner["original_username"]} + api_url = self.get_api_url("api/edxnotes/v1/retire_user") + return self._request("POST", api_url, json=data) @_retry_lms_api() def retirement_lms_retire_misc(self, learner): @@ -304,27 +296,27 @@ def retirement_lms_retire_misc(self, learner): Deletes, blanks, or one-way hashes personal information in LMS as defined in EDUCATOR-2802 and sub-tasks. """ - data = {'username': learner['original_username']} - api_url = self.get_api_url('api/user/v1/accounts/retire_misc') - return self._request('POST', api_url, json=data) + data = {"username": learner["original_username"]} + api_url = self.get_api_url("api/user/v1/accounts/retire_misc") + return self._request("POST", api_url, json=data) @_retry_lms_api() def retirement_lms_retire(self, learner): """ Deletes, blanks, or one-way hashes all remaining personal information in LMS """ - data = {'username': learner['original_username']} - api_url = self.get_api_url('api/user/v1/accounts/retire') - return self._request('POST', api_url, json=data) + data = {"username": learner["original_username"]} + api_url = self.get_api_url("api/user/v1/accounts/retire") + return self._request("POST", api_url, json=data) @_retry_lms_api() def retirement_partner_queue(self, learner): """ Calls LMS to add the given user to the retirement reporting queue """ - data = {'username': learner['original_username']} - api_url = self.get_api_url('api/user/v1/accounts/retirement_partner_report') - return self._request('PUT', api_url, json=data) + data = {"username": learner["original_username"]} + api_url = self.get_api_url("api/user/v1/accounts/retirement_partner_report") + return self._request("PUT", api_url, json=data) @_retry_lms_api() def retirement_partner_report(self): @@ -332,16 +324,16 @@ def retirement_partner_report(self): Retrieves the list of users to create partner reports for and set their status to processing """ - api_url = self.get_api_url('api/user/v1/accounts/retirement_partner_report') - return self._request('POST', api_url) + api_url = self.get_api_url("api/user/v1/accounts/retirement_partner_report") + return self._request("POST", api_url) @_retry_lms_api() def retirement_partner_cleanup(self, usernames): """ Removes the given users from the partner reporting queue """ - api_url = self.get_api_url('api/user/v1/accounts/retirement_partner_report_cleanup') - return self._request('POST', api_url, json=usernames) + api_url = self.get_api_url("api/user/v1/accounts/retirement_partner_report_cleanup") + return self._request("POST", api_url, json=usernames) @_retry_lms_api() def retirement_retire_proctoring_data(self, learner): @@ -349,7 +341,7 @@ def retirement_retire_proctoring_data(self, learner): Deletes or hashes learner data from edx-proctoring """ api_url = self.get_api_url(f"api/edx_proctoring/v1/retire_user/{learner['user']['id']}") - return self._request('POST', api_url) + return self._request("POST", api_url) @_retry_lms_api() def retirement_retire_proctoring_backend_data(self, learner): @@ -357,16 +349,16 @@ def retirement_retire_proctoring_backend_data(self, learner): Removes the given learner from 3rd party proctoring backends """ api_url = self.get_api_url(f"api/edx_proctoring/v1/retire_backend_user/{learner['user']['id']}") - return self._request('POST', api_url) + return self._request("POST", api_url) @_retry_lms_api() def bulk_cleanup_retirements(self, usernames): """ Deletes the retirements for all given usernames """ - data = {'usernames': usernames} - api_url = self.get_api_url('api/user/v1/accounts/retirement_cleanup') - return self._request('POST', api_url, json=data) + data = {"usernames": usernames} + api_url = self.get_api_url("api/user/v1/accounts/retirement_cleanup") + return self._request("POST", api_url, json=data) def replace_lms_usernames(self, username_mappings): """ @@ -377,8 +369,8 @@ def replace_lms_usernames(self, username_mappings): [{current_un_1: desired_un_1}, {current_un_2: desired_un_2}] """ data = {"username_mappings": username_mappings} - api_url = self.get_api_url('api/user/v1/accounts/replace_usernames') - return self._request('POST', api_url, json=data) + api_url = self.get_api_url("api/user/v1/accounts/replace_usernames") + return self._request("POST", api_url, json=data) def replace_forums_usernames(self, username_mappings): """ @@ -389,8 +381,8 @@ def replace_forums_usernames(self, username_mappings): [{current_un_1: new_un_1}, {current_un_2: new_un_2}] """ data = {"username_mappings": username_mappings} - api_url = self.get_api_url('api/discussion/v1/accounts/replace_usernames') - return self._request('POST', api_url, json=data) + api_url = self.get_api_url("api/discussion/v1/accounts/replace_usernames") + return self._request("POST", api_url, json=data) class EcommerceApi(BaseApiClient): @@ -403,9 +395,9 @@ def retire_learner(self, learner): """ Performs the learner retirement step for Ecommerce """ - data = {'username': learner['original_username']} - api_url = self.get_api_url('api/v2/user/retire') - return self._request('POST', api_url, json=data) + data = {"username": learner["original_username"]} + api_url = self.get_api_url("api/v2/user/retire") + return self._request("POST", api_url, json=data) @_retry_lms_api() def get_tracking_key(self, learner): @@ -414,7 +406,7 @@ def get_tracking_key(self, learner): ecommerce doesn't have access to the LMS user id. """ api_url = self.get_api_url(f"api/v2/retirement/tracking_id/{learner['original_username']}") - return self._request('GET', api_url)['ecommerce_tracking_id'] + return self._request("GET", api_url)["ecommerce_tracking_id"] def replace_usernames(self, username_mappings): """ @@ -425,8 +417,8 @@ def replace_usernames(self, username_mappings): [{current_un_1: new_un_1}, {current_un_2: new_un_2}] """ data = {"username_mappings": username_mappings} - api_url = self.get_api_url('api/v2/user_management/replace_usernames') - return self._request('POST', api_url, json=data) + api_url = self.get_api_url("api/v2/user_management/replace_usernames") + return self._request("POST", api_url, json=data) class CredentialsApi(BaseApiClient): @@ -439,9 +431,9 @@ def retire_learner(self, learner): """ Performs the learner retirement step for Credentials """ - data = {'username': learner['original_username']} - api_url = self.get_api_url('user/retire') - return self._request('POST', api_url, json=data) + data = {"username": learner["original_username"]} + api_url = self.get_api_url("user/retire") + return self._request("POST", api_url, json=data) def replace_usernames(self, username_mappings): """ @@ -452,8 +444,8 @@ def replace_usernames(self, username_mappings): [{current_un_1: new_un_1}, {current_un_2: new_un_2}] """ data = {"username_mappings": username_mappings} - api_url = self.get_api_url('api/v2/replace_usernames') - return self._request('POST', api_url, json=data) + api_url = self.get_api_url("api/v2/replace_usernames") + return self._request("POST", api_url, json=data) class DiscoveryApi(BaseApiClient): @@ -470,30 +462,8 @@ def replace_usernames(self, username_mappings): [{current_un_1: new_un_1}, {current_un_2: new_un_2}] """ data = {"username_mappings": username_mappings} - api_url = self.get_api_url('api/v1/replace_usernames') - return self._request('POST', api_url, json=data) - - -class DemographicsApi(BaseApiClient): - """ - Demographics API client. - """ - - @_retry_lms_api() - def retire_learner(self, learner): - """ - Performs the learner retirement step for Demographics. Passes the learner's LMS User Id instead of username. - """ - data = {'lms_user_id': learner['user']['id']} - # If the user we are retiring has no data in the Demographics DB the request will return a 404. We - # catch the HTTPError and return True in order to prevent this error getting raised and - # incorrectly causing the learner to enter an ERROR state during retirement. - try: - api_url = self.get_api_url('demographics/api/v1/retire_demographics') - return self._request('POST', api_url, log_404_as_error=False, json=data) - except HttpDoesNotExistException: - LOG.info("No demographics data found for user") - return True + api_url = self.get_api_url("api/v1/replace_usernames") + return self._request("POST", api_url, json=data) class LicenseManagerApi(BaseApiClient): @@ -508,15 +478,15 @@ def retire_learner(self, learner): username. """ data = { - 'lms_user_id': learner['user']['id'], - 'original_username': learner['original_username'], + "lms_user_id": learner["user"]["id"], + "original_username": learner["original_username"], } # If the user we are retiring has no data in the License Manager DB the request will return a 404. We # catch the HTTPError and return True in order to prevent this error getting raised and # incorrectly causing the learner to enter an ERROR state during retirement. try: - api_url = self.get_api_url('api/v1/retire_user') - return self._request('POST', api_url, log_404_as_error=False, json=data) + api_url = self.get_api_url("api/v1/retire_user") + return self._request("POST", api_url, log_404_as_error=False, json=data) except HttpDoesNotExistException: LOG.info("No license manager data found for user") return True