FC-0001: Remove EdxRestApiClient usage in edx-analatycis-data-api

analytics_data_api/management/commands/generate_data.py

@@ -298,14 +298,12 @@ def generate_tags_distribution_data(course_id, database):
 
 def fetch_videos_from_course_blocks(course_id):
     logger.info("Fetching video ids from Course Blocks API...")
-    try:
-        api_base_url = settings.LMS_BASE_URL + 'api/courses/v1/'
-    except AttributeError:
-        logger.warning("LMS_BASE_URL is not configured! Cannot get video ids.")
-        return None
-    logger.info("Assuming the Course Blocks API is hosted at: %s", api_base_url)
-
-    blocks_api = CourseBlocksApiClient(api_base_url, settings.COURSE_BLOCK_API_AUTH_TOKEN, timeout=5)
+
+    blocks_api = CourseBlocksApiClient(
+        settings.BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL,
+        settings.BACKEND_SERVICE_EDX_OAUTH2_KEY,
+        settings.BACKEND_SERVICE_EDX_OAUTH2_SECRET,
+    )
     return blocks_api.all_videos(course_id)
 
 

analyticsdataserver/clients.py

@@ -1,17 +1,18 @@
 import logging
+from urllib.parse import urljoin
 
-from edx_rest_api_client.client import EdxRestApiClient
-from edx_rest_api_client.exceptions import HttpClientError
+from django.conf import settings
+from edx_rest_api_client.client import OAuthAPIClient
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import UsageKey
-from requests.exceptions import RequestException
+from requests.exceptions import HTTPError, RequestException
 
 from analyticsdataserver.utils import temp_log_level
 
 logger = logging.getLogger(__name__)
 
 
-class CourseBlocksApiClient(EdxRestApiClient):
+class CourseBlocksApiClient(OAuthAPIClient):
     """
     This class is a sub-class of the edX Rest API Client
     (https://github.com/edx/edx-rest-api-client).
@@ -22,15 +23,28 @@ class CourseBlocksApiClient(EdxRestApiClient):
     Currently, this client is only used for a local-only developer script (generate_fake_course_data).
     """
 
-    def __init__(self, url, access_token, timeout):
-        super().__init__(url, oauth_access_token=access_token, timeout=timeout)
-
     def all_videos(self, course_id):
         try:
             logger.debug('Retrieving course video blocks for course_id: %s', course_id)
-            response = self.blocks.get(course_id=course_id, all_blocks=True, depth='all', block_types_filter='video')
+
+            try:
+                api_base_url = urljoin(settings.LMS_BASE_URL + '/', 'api/courses/v1/')
+            except AttributeError:
+                logger.warning("LMS_BASE_URL is not configured! Cannot get video ids.")
+                return None
+            logger.info("Assuming the Course Blocks API is hosted at: %s", api_base_url)
+
+            blocks_kwargs = {
+                'course_id': course_id,
+                'all_blocks': True,
+                'depth': 'all',
+                'block_types_filter': 'video'
+            }
+            response = self.get(urljoin(api_base_url, 'blocks/'), params=blocks_kwargs)
+            response.raise_for_status()
+            data = response.json()
             logger.info("Successfully authenticated with the Course Blocks API.")
-        except HttpClientError as e:
+        except HTTPError as e:
             if e.response.status_code == 401:
                 logger.warning("Course Blocks API failed to return video ids (%s). "
                                "See README for instructions on how to authenticate the API with your local LMS.",
@@ -50,7 +64,7 @@ def all_videos(self, course_id):
         # (The UsageKey utility still works despite the import errors, so I think the errors are not important).
         with temp_log_level('stevedore', log_level=logging.CRITICAL):
             videos = []
-            for video in response['blocks'].values():
+            for video in data['blocks'].values():
                 try:
                     encoded_id = UsageKey.from_string(video['id']).html_id()
                 except InvalidKeyError:

analyticsdataserver/settings/local.py

@@ -85,7 +85,6 @@
 # These two settings are used in generate_fake_course_data.py.
 # Replace with correct values to generate local fake video data.
 LMS_BASE_URL = 'http://localhost:18000/'  # the base URL for your running local LMS instance
-COURSE_BLOCK_API_AUTH_TOKEN = 'paste auth token here'  # see README for instructions on how to configure this value
 
 # In Insights, we run this API as a separate service called "analyticsapi" to run acceptance/integration tests. Docker
 # saves the service name as a host in the Insights container so it can reach the API by requesting http://analyticsapi/.

analyticsdataserver/tests/test_clients.py

@@ -1,28 +1,39 @@
-import json
 import unittest.mock as mock
+from urllib.parse import urljoin
 
 import responses
+from django.conf import settings
 from django.test import TestCase
 from requests.exceptions import ConnectionError as RequestsConnectionError
 
 from analyticsdataserver.clients import CourseBlocksApiClient
 
 
 class ClientTests(TestCase):
-    @mock.patch('analyticsdataserver.clients.EdxRestApiClient')
-    def setUp(self, restApiClientMock):
-        self.client = CourseBlocksApiClient('http://example.com/', 'token', 5)
+
+    def setUp(self):
+        self.client = CourseBlocksApiClient(settings.LMS_BASE_URL, 'client_id', 'client_secret')
+        responses.add(
+            responses.POST,
+            urljoin(settings.LMS_BASE_URL, 'oauth2/access_token'),
+            status=200,
+            json={
+                'access_token': 'test_access_token',
+                'expires_in': 10,
+            },
+            content_type='application/json'
+        )
 
     @responses.activate
     def test_all_videos(self):
-        responses.add(responses.GET, 'http://example.com/blocks/', body=json.dumps({'blocks': {
+        responses.add(responses.GET, urljoin(settings.LMS_BASE_URL, 'api/courses/v1/blocks/'), json={'blocks': {
             'block-v1:edX+DemoX+Demo_Course+type@video+block@5c90cffecd9b48b188cbfea176bf7fe9': {
                 'id': 'block-v1:edX+DemoX+Demo_Course+type@video+block@5c90cffecd9b48b188cbfea176bf7fe9'
             },
             'block-v1:edX+DemoX+Demo_Course+type@video+block@7e9b434e6de3435ab99bd3fb25bde807': {
                 'id': 'block-v1:edX+DemoX+Demo_Course+type@video+block@7e9b434e6de3435ab99bd3fb25bde807'
             }
-        }}), status=200, content_type='application/json')
+        }}, status=200, content_type='application/json')
         videos = self.client.all_videos('course_id')
         self.assertListEqual(videos, [
             {
@@ -38,7 +49,7 @@ def test_all_videos(self):
     @responses.activate
     @mock.patch('analyticsdataserver.clients.logger')
     def test_all_videos_401(self, logger):
-        responses.add(responses.GET, 'http://example.com/blocks/', status=401, content_type='application/json')
+        responses.add(responses.GET, urljoin(settings.LMS_BASE_URL, 'api/courses/v1/blocks/'), status=401, content_type='application/json')
         videos = self.client.all_videos('course_id')
         logger.warning.assert_called_with(
             'Course Blocks API failed to return video ids (%s). '
@@ -48,7 +59,7 @@ def test_all_videos_401(self, logger):
     @responses.activate
     @mock.patch('analyticsdataserver.clients.logger')
     def test_all_videos_404(self, logger):
-        responses.add(responses.GET, 'http://example.com/blocks/', status=404, content_type='application/json')
+        responses.add(responses.GET, urljoin(settings.LMS_BASE_URL, 'api/courses/v1/blocks/'), status=404, content_type='application/json')
         videos = self.client.all_videos('course_id')
         logger.warning.assert_called_with('Course Blocks API failed to return video ids (%s). '
                                           'Does the course exist in the LMS?', 404)
@@ -57,7 +68,7 @@ def test_all_videos_404(self, logger):
     @responses.activate
     @mock.patch('analyticsdataserver.clients.logger')
     def test_all_videos_500(self, logger):
-        responses.add(responses.GET, 'http://example.com/blocks/', status=418, content_type='application/json')
+        responses.add(responses.GET, urljoin(settings.LMS_BASE_URL, 'api/courses/v1/blocks/'), status=418, content_type='application/json')
         videos = self.client.all_videos('course_id')
         logger.warning.assert_called_with('Course Blocks API failed to return video ids (%s).', 418)
         self.assertEqual(videos, None)
@@ -66,22 +77,22 @@ def test_all_videos_500(self, logger):
     @mock.patch('analyticsdataserver.clients.logger')
     def test_all_videos_connection_error(self, logger):
         exception = RequestsConnectionError('LMS is dead')
-        responses.add(responses.GET, 'http://example.com/blocks/', body=exception)
+        responses.add(responses.GET, urljoin(settings.LMS_BASE_URL, 'api/courses/v1/blocks/'), body=exception)
         videos = self.client.all_videos('course_id')
         logger.warning.assert_called_with('Course Blocks API request failed. Is the LMS running?: %s', str(exception))
         self.assertEqual(videos, None)
 
     @responses.activate
     def test_all_videos_pass_through_bad_id(self):
-        responses.add(responses.GET, 'http://example.com/blocks/', body=json.dumps({'blocks': {
+        responses.add(responses.GET, urljoin(settings.LMS_BASE_URL, 'api/courses/v1/blocks/'), json={'blocks': {
             'block-v1:edX+DemoX+Demo_Course+type@video+block@5c90cffecd9b48b188cbfea176bf7fe9': {
                 'id': 'bad_key'
             },
             'block-v1:edX+DemoX+Demo_Course+type@video+block@7e9b434e6de3435ab99bd3fb25bde807': {
                 'id': 'bad_key'
             }
-        }}), status=200, content_type='application/json')
-        responses.add(responses.GET, 'http://example.com/blocks/', status=200, content_type='application/json')
+        }}, status=200, content_type='application/json')
+        responses.add(responses.GET, urljoin(settings.LMS_BASE_URL, 'api/courses/v1/blocks/'), status=200, content_type='application/json')
         videos = self.client.all_videos('course_id')
         self.assertListEqual(videos, [
             {