From 2aa00f97dc5558c85d8cf34d68f4539d096578bc Mon Sep 17 00:00:00 2001 From: Yagnesh Date: Mon, 6 Nov 2023 21:12:35 +0530 Subject: [PATCH] feat: DEPR EdxRestApiClient --- CHANGELOG.rst | 4 + edx_rest_api_client/client.py | 71 +---------- edx_rest_api_client/exceptions.py | 1 - edx_rest_api_client/tests/test_client.py | 143 +---------------------- requirements/base.in | 1 - requirements/base.txt | 4 - requirements/common_constraints.txt | 14 +-- requirements/dev.txt | 7 +- requirements/test.txt | 3 - 9 files changed, 17 insertions(+), 231 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d21cbec..f894335 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,10 @@ Unreleased ---------- * Nothing +[6.0.0] +-------- +Breaking Changes: The EdxRestApiClient` has been deprecated and removed in this release. + [5.7.1] -------- chore: Update Requirements specifically to unpin the requests library diff --git a/edx_rest_api_client/client.py b/edx_rest_api_client/client.py index f738508..8f58a7a 100644 --- a/edx_rest_api_client/client.py +++ b/edx_rest_api_client/client.py @@ -1,17 +1,16 @@ import datetime import json -import os import socket +import os import crum import requests import requests.utils -import slumber from edx_django_utils.cache import TieredCache from edx_django_utils.monitoring import set_custom_attribute from edx_rest_api_client.__version__ import __version__ -from edx_rest_api_client.auth import BearerAuth, JwtAuth, SuppliedJwtAuth +from edx_rest_api_client.auth import SuppliedJwtAuth # When caching tokens, use this value to err on expiring tokens a little early so they are # sure to be valid at the time they are used. @@ -299,69 +298,3 @@ def request(self, method, url, headers=None, **kwargs): # pylint: disable=argum set_custom_attribute('api_client', 'OAuthAPIClient') self._ensure_authentication() return super().request(method, url, headers=headers, **kwargs) - - -class EdxRestApiClient(slumber.API): - """ - API client for edX REST API. - - (deprecated) See docs/decisions/0002-oauth-api-client-replacement.rst. - """ - - @classmethod - def user_agent(cls): - return USER_AGENT - - @classmethod - def get_oauth_access_token(cls, url, client_id, client_secret, token_type='bearer', - timeout=(REQUEST_CONNECT_TIMEOUT, REQUEST_READ_TIMEOUT)): - # 'To help transition to OAuthAPIClient, use EdxRestApiClient.get_and_cache_jwt_oauth_access_token instead' - # 'of EdxRestApiClient.get_oauth_access_token to share cached jwt token used by OAuthAPIClient.' - return get_oauth_access_token(url, client_id, client_secret, token_type=token_type, timeout=timeout) - - @classmethod - def get_and_cache_jwt_oauth_access_token(cls, url, client_id, client_secret, - timeout=(REQUEST_CONNECT_TIMEOUT, REQUEST_READ_TIMEOUT)): - return get_and_cache_oauth_access_token(url, client_id, client_secret, token_type="jwt", timeout=timeout) - - def __init__(self, url, signing_key=None, username=None, full_name=None, email=None, - timeout=5, issuer=None, expires_in=30, tracking_context=None, oauth_access_token=None, - session=None, jwt=None, **kwargs): - """ - EdxRestApiClient is deprecated. Use OAuthAPIClient instead. - - Instantiate a new client. You can pass extra kwargs to Slumber like - 'append_slash'. - - Raises: - ValueError: If a URL is not provided. - - """ - set_custom_attribute('api_client', 'EdxRestApiClient') - if not url: - raise ValueError('An API url must be supplied!') - - if jwt: - auth = SuppliedJwtAuth(jwt) - elif oauth_access_token: - auth = BearerAuth(oauth_access_token) - elif signing_key and username: - auth = JwtAuth(username, full_name, email, signing_key, - issuer=issuer, expires_in=expires_in, tracking_context=tracking_context) - else: - auth = None - - session = session or requests.Session() - session.headers['User-Agent'] = self.user_agent() - - session.timeout = timeout - super().__init__( - url, - session=session, - auth=auth, - **kwargs - ) - - -EdxRestApiClient.user_agent.__func__.__doc__ = user_agent.__doc__ -EdxRestApiClient.get_oauth_access_token.__func__.__doc__ = get_oauth_access_token.__doc__ diff --git a/edx_rest_api_client/exceptions.py b/edx_rest_api_client/exceptions.py index 068d8c1..7d29fc9 100644 --- a/edx_rest_api_client/exceptions.py +++ b/edx_rest_api_client/exceptions.py @@ -1,3 +1,2 @@ # noinspection PyUnresolvedReferences from requests.exceptions import Timeout # pylint: disable=unused-import -from slumber.exceptions import * # pylint: disable=wildcard-import diff --git a/edx_rest_api_client/tests/test_client.py b/edx_rest_api_client/tests/test_client.py index cfae193..f2cb9a0 100644 --- a/edx_rest_api_client/tests/test_client.py +++ b/edx_rest_api_client/tests/test_client.py @@ -1,6 +1,5 @@ import datetime import json -import os from unittest import TestCase, mock import ddt @@ -10,9 +9,8 @@ from freezegun import freeze_time from edx_rest_api_client import __version__ -from edx_rest_api_client.auth import JwtAuth -from edx_rest_api_client.client import (EdxRestApiClient, OAuthAPIClient, get_and_cache_oauth_access_token, - get_oauth_access_token, user_agent) +from edx_rest_api_client.client import (OAuthAPIClient, get_and_cache_oauth_access_token, + get_oauth_access_token) from edx_rest_api_client.tests.mixins import AuthenticationTestMixin URL = 'http://example.com/api/v2' @@ -27,124 +25,12 @@ JWT = 'abc.123.doremi' -@ddt.ddt -class EdxRestApiClientTests(TestCase): - """ - Tests for the edX Rest API client. - """ - - @ddt.unpack - @ddt.data( - ({'url': URL, 'signing_key': SIGNING_KEY, 'username': USERNAME, - 'full_name': FULL_NAME, 'email': EMAIL}, JwtAuth), - ({'url': URL, 'signing_key': SIGNING_KEY, 'username': USERNAME, 'full_name': None, 'email': EMAIL}, JwtAuth), - ({'url': URL, 'signing_key': SIGNING_KEY, 'username': USERNAME, - 'full_name': FULL_NAME, 'email': None}, JwtAuth), - ({'url': URL, 'signing_key': SIGNING_KEY, 'username': USERNAME, 'full_name': None, 'email': None}, JwtAuth), - ({'url': URL, 'signing_key': SIGNING_KEY, 'username': USERNAME}, JwtAuth), - ({'url': URL, 'signing_key': None, 'username': USERNAME}, type(None)), - ({'url': URL, 'signing_key': SIGNING_KEY, 'username': None}, type(None)), - ({'url': URL, 'signing_key': None, 'username': None, 'oauth_access_token': None}, type(None)) - ) - def test_valid_configuration(self, kwargs, auth_type): - """ - The constructor should return successfully if all arguments are valid. - We also check that the auth type of the api is what we expect. - """ - api = EdxRestApiClient(**kwargs) - self.assertIsInstance(api._store['session'].auth, auth_type) # pylint: disable=protected-access - - @ddt.data( - {'url': None, 'signing_key': SIGNING_KEY, 'username': USERNAME}, - {'url': None, 'signing_key': None, 'username': None, 'oauth_access_token': None}, - ) - def test_invalid_configuration(self, kwargs): - """ - If the constructor arguments are invalid, an InvalidConfigurationError should be raised. - """ - self.assertRaises(ValueError, EdxRestApiClient, **kwargs) - - @mock.patch('edx_rest_api_client.auth.JwtAuth.__init__', return_value=None) - def test_tracking_context(self, mock_auth): - """ - Ensure the tracking context is included with API requests if specified. - """ - EdxRestApiClient(URL, SIGNING_KEY, USERNAME, FULL_NAME, EMAIL, tracking_context=TRACKING_CONTEXT) - self.assertIn(TRACKING_CONTEXT, mock_auth.call_args[1].values()) - - def test_oauth2(self): - """ - Ensure OAuth2 authentication is used when an access token is supplied to the constructor. - """ - - with mock.patch('edx_rest_api_client.auth.BearerAuth.__init__', return_value=None) as mock_auth: - EdxRestApiClient(URL, oauth_access_token=ACCESS_TOKEN) - mock_auth.assert_called_with(ACCESS_TOKEN) - - def test_supplied_jwt(self): - """Ensure JWT authentication is used when a JWT is supplied to the constructor.""" - with mock.patch('edx_rest_api_client.auth.SuppliedJwtAuth.__init__', return_value=None) as mock_auth: - EdxRestApiClient(URL, jwt=JWT) - mock_auth.assert_called_with(JWT) - - def test_user_agent(self): - """Make sure our custom User-Agent is getting built correctly.""" - with mock.patch('socket.gethostbyname', return_value='test_hostname'): - default_user_agent = user_agent() - self.assertIn('python-requests', default_user_agent) - self.assertIn(f'edx-rest-api-client/{__version__}', default_user_agent) - self.assertIn('test_hostname', default_user_agent) - - with mock.patch('socket.gethostbyname') as mock_gethostbyname: - mock_gethostbyname.side_effect = ValueError() - default_user_agent = user_agent() - self.assertIn('unknown_client_name', default_user_agent) - - with mock.patch.dict(os.environ, {'EDX_REST_API_CLIENT_NAME': "awesome_app"}): - uagent = user_agent() - self.assertIn('awesome_app', uagent) - - self.assertEqual(user_agent(), EdxRestApiClient.user_agent()) - - @ddt.ddt class ClientCredentialTests(AuthenticationTestMixin, TestCase): """ Test client credentials requests. """ - @responses.activate - def test_get_client_credential_access_token_success(self): - """ - Test that the get access token method handles 200 responses and returns the access token. - """ - code = 200 - body = {"access_token": "my-token", "expires_in": 1000} - now = datetime.datetime.utcnow() - - expected_return = ("my-token", now + datetime.timedelta(seconds=1000)) - - with freeze_time(now): - self._mock_auth_api(OAUTH_URL, code, body=body) - self.assertEqual( - EdxRestApiClient.get_oauth_access_token(OAUTH_URL, "client_id", "client_secret"), - expected_return - ) - - @ddt.data( - (400, {"error": "denied"}), - (500, None) - ) - @ddt.unpack - @responses.activate - def test_get_client_credential_access_token_failure(self, code, body): - """ - Test that the get access token method handles failure responses. - """ - with self.assertRaises(requests.RequestException): - self._mock_auth_api(OAUTH_URL, code, body=body) - EdxRestApiClient.get_oauth_access_token(OAUTH_URL, "client_id", "client_secret") - def test_refresh_token_required(self): self._mock_auth_api(OAUTH_URL, 200, body=None) with self.assertRaises(AssertionError): @@ -160,31 +46,6 @@ def setUp(self): super().setUp() TieredCache.dangerous_clear_all_tiers() - @responses.activate - def test_shared_client_credential_jwt_access_token(self): - """ - Test that get_and_cache_jwt_oauth_access_token returns the same access token used by the OAuthAPIClient. - """ - body = {'access_token': "my-token", 'expires_in': 1000} - now = datetime.datetime.utcnow() - expected_return = ('my-token', now + datetime.timedelta(seconds=1000)) - - with freeze_time(now): - self._mock_auth_api(OAUTH_URL, 200, body=body) - actual_return = EdxRestApiClient.get_and_cache_jwt_oauth_access_token( - OAUTH_URL, 'client_id', 'client_secret' - ) - self.assertEqual(actual_return, expected_return) - self.assertEqual(len(responses.calls), 1) - - # ensure OAuthAPIClient uses the same cached auth token without re-requesting the token from the server - oauth_client = OAuthAPIClient(OAUTH_URL, 'client_id', 'client_secret') - self._mock_auth_api(URL, 200, {'status': 'ok'}) - oauth_client.post(URL, data={'test': 'ok'}) - self.assertEqual(oauth_client.auth.token, actual_return[0]) - self.assertEqual(len(responses.calls), 2) - self.assertEqual(URL, responses.calls[1][0].url) - @responses.activate def test_token_caching(self): """ diff --git a/requirements/base.in b/requirements/base.in index 58e5f38..01e2f4b 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -3,5 +3,4 @@ edx-django-utils requests -slumber PyJWT diff --git a/requirements/base.txt b/requirements/base.txt index 6ba97f1..5f5c863 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -45,10 +45,6 @@ pyjwt==2.8.0 pynacl==1.5.0 # via edx-django-utils requests==2.32.3 - # via - # -r requirements/base.in - # slumber -slumber==0.7.1 # via -r requirements/base.in sqlparse==0.5.0 # via django diff --git a/requirements/common_constraints.txt b/requirements/common_constraints.txt index 22beac8..5ba9104 100644 --- a/requirements/common_constraints.txt +++ b/requirements/common_constraints.txt @@ -22,15 +22,15 @@ Django<5.0 # elasticsearch>=7.14.0 includes breaking changes in it which caused issues in discovery upgrade process. # elastic search changelog: https://www.elastic.co/guide/en/enterprise-search/master/release-notes-7.14.0.html +# See https://github.com/openedx/edx-platform/issues/35126 for more info elasticsearch<7.14.0 # django-simple-history>3.0.0 adds indexing and causes a lot of migrations to be affected django-simple-history==3.0.0 -# opentelemetry requires version 6.x at the moment: -# https://github.com/open-telemetry/opentelemetry-python/issues/3570 -# Normally this could be added as a constraint in edx-django-utils, where we're -# adding the opentelemetry dependency. However, when we compile pip-tools.txt, -# that uses version 7.x, and then there's no undoing that when compiling base.txt. -# So we need to pin it globally, for now. -# Ticket for unpinning: https://github.com/openedx/edx-lint/issues/407 +# Cause: https://github.com/openedx/event-tracking/pull/290 +# event-tracking 2.4.1 upgrades to pymongo 4.4.0 which is not supported on edx-platform. +# We will pin event-tracking to do not break existing installations +# This can be unpinned once https://github.com/openedx/edx-platform/issues/34586 +# has been resolved and edx-platform is running with pymongo>=4.4.0 +event-tracking<2.4.1 diff --git a/requirements/dev.txt b/requirements/dev.txt index f425b2c..b17ed85 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -113,7 +113,7 @@ exceptiongroup==1.2.1 # via # -r requirements/test.txt # pytest -filelock==3.14.0 +filelock==3.15.4 # via # -r requirements/ci.txt # tox @@ -311,7 +311,6 @@ requests==2.32.3 # -r requirements/test.txt # requests-toolbelt # responses - # slumber # twine requests-toolbelt==1.0.0 # via @@ -336,8 +335,6 @@ six==1.16.0 # -r requirements/test.txt # edx-lint # python-dateutil -slumber==0.7.1 - # via -r requirements/test.txt sqlparse==0.5.0 # via # -r requirements/test.txt @@ -384,7 +381,7 @@ urllib3==2.2.2 # requests # responses # twine -virtualenv==20.26.2 +virtualenv==20.26.3 # via # -r requirements/ci.txt # tox diff --git a/requirements/test.txt b/requirements/test.txt index 8d04bf0..ce7188c 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -192,7 +192,6 @@ requests==2.32.3 # -r requirements/base.txt # requests-toolbelt # responses - # slumber # twine requests-toolbelt==1.0.0 # via twine @@ -208,8 +207,6 @@ six==1.16.0 # via # edx-lint # python-dateutil -slumber==0.7.1 - # via -r requirements/base.txt sqlparse==0.5.0 # via # -r requirements/base.txt