feat: DEPR EdxRestApiClient

openedx · Sep 5, 2024 · 2aa00f9 · 2aa00f9
1 parent c11abde
commit 2aa00f9
Show file tree

Hide file tree

Showing 9 changed files with 17 additions and 231 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -3,5 +3,4 @@
 
 edx-django-utils
 requests
-slumber
 PyJWT
diff --git 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
@@ -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
@@ -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
@@ -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
-Original file line number
+Diff line change
@@ Expand Up / @@ -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
@@ Expand Down @@