refactor: Made preview and announcements configurable through env (#740)

* Made preview and announcements configurable through environment variables
Signed-off-by: Josh Howard <joshthoward@gmail.com>

* Fixed deprecation warnings in tests
Signed-off-by: Josh Howard <joshthoward@gmail.com>

* Replaced backward compatibility for entrypoint config
Signed-off-by: Josh Howard <joshthoward@gmail.com>

* Added deprecation warnings for entrypoint config
Signed-off-by: Josh Howard <joshthoward@gmail.com>
Co-authored-by: Josh Howard <josh.t.howard@ey.com>

amundsen_application/api/announcements/v0.py

@@ -2,20 +2,18 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import logging
+from pkg_resources import iter_entry_points
 
 from http import HTTPStatus
-from pkg_resources import iter_entry_points
 
-from flask import Response, jsonify, make_response
+from flask import Response, jsonify, make_response, current_app as app
 from flask.blueprints import Blueprint
+from werkzeug.utils import import_string
 
 LOGGER = logging.getLogger(__name__)
-
-# TODO: Blueprint classes might be the way to go
 ANNOUNCEMENT_CLIENT_CLASS = None
 ANNOUNCEMENT_CLIENT_INSTANCE = None
 
-# get the announcement_client_class from the python entry point
 for entry_point in iter_entry_points(group='announcement_client', name='announcement_client_class'):
     announcement_client_class = entry_point.load()
     if announcement_client_class is not None:
@@ -27,14 +25,21 @@ announcements_blueprint = Blueprint('announcements', __name__, url_prefix='/api/
 @announcements_blueprint.route('/', methods=['GET'])
 def get_announcements() -> Response:
     global ANNOUNCEMENT_CLIENT_INSTANCE
+    global ANNOUNCEMENT_CLIENT_CLASS
     try:
-        if ANNOUNCEMENT_CLIENT_INSTANCE is None and ANNOUNCEMENT_CLIENT_CLASS is not None:
-            ANNOUNCEMENT_CLIENT_INSTANCE = ANNOUNCEMENT_CLIENT_CLASS()
-
         if ANNOUNCEMENT_CLIENT_INSTANCE is None:
-            payload = jsonify({'posts': [], 'msg': 'A client for retrieving announcements must be configured'})
+            if ANNOUNCEMENT_CLIENT_CLASS is not None:
+                ANNOUNCEMENT_CLIENT_INSTANCE = ANNOUNCEMENT_CLIENT_CLASS()
+                logging.warn('Setting announcement_client via entry_point is DEPRECATED'
+                             ' and will be removed in a future version')
+            elif (app.config['ANNOUNCEMENT_CLIENT_ENABLED']
+                    and app.config['ANNOUNCEMENT_CLIENT'] is not None):
+                ANNOUNCEMENT_CLIENT_CLASS = import_string(app.config['ANNOUNCEMENT_CLIENT'])
+                ANNOUNCEMENT_CLIENT_INSTANCE = ANNOUNCEMENT_CLIENT_CLASS()
+            else:
+                payload = jsonify({'posts': [],
+                                   'msg': 'A client for retrieving announcements must be configured'})
                 return make_response(payload, HTTPStatus.NOT_IMPLEMENTED)
-
         return ANNOUNCEMENT_CLIENT_INSTANCE._get_posts()
     except Exception as e:
         message = 'Encountered exception: ' + str(e)

amundsen_application/api/preview/v0.py

@@ -3,22 +3,20 @@
 
 import json
 import logging
+from pkg_resources import iter_entry_points
 
 from http import HTTPStatus
-from pkg_resources import iter_entry_points
 
-from flask import Response, jsonify, make_response, request
+from flask import Response, jsonify, make_response, request, current_app as app
 from flask.blueprints import Blueprint
+from werkzeug.utils import import_string
 
 from amundsen_application.models.preview_data import PreviewDataSchema
 
 LOGGER = logging.getLogger(__name__)
-
-# TODO: Blueprint classes might be the way to go
 PREVIEW_CLIENT_CLASS = None
 PREVIEW_CLIENT_INSTANCE = None
 
-# get the preview_client_class from the python entry point
 for entry_point in iter_entry_points(group='preview_client', name='table_preview_client_class'):
     preview_client_class = entry_point.load()
     if preview_client_class is not None:
@@ -29,17 +27,22 @@ preview_blueprint = Blueprint('preview', __name__, url_prefix='/api/preview/v0')
 
 @preview_blueprint.route('/', methods=['POST'])
 def get_table_preview() -> Response:
-    # TODO: Want to further separate this file into more blueprints, perhaps a Blueprint class is the way to go
     global PREVIEW_CLIENT_INSTANCE
+    global PREVIEW_CLIENT_CLASS
     try:
-        if PREVIEW_CLIENT_INSTANCE is None and PREVIEW_CLIENT_CLASS is not None:
-            PREVIEW_CLIENT_INSTANCE = PREVIEW_CLIENT_CLASS()
-
         if PREVIEW_CLIENT_INSTANCE is None:
+            if PREVIEW_CLIENT_CLASS is not None:
+                PREVIEW_CLIENT_INSTANCE = PREVIEW_CLIENT_CLASS()
+                logging.warn('Setting preview_client via entry_point is DEPRECATED and '
+                             'will be removed in a future version')
+            elif (app.config['PREVIEW_CLIENT_ENABLED']
+                    and app.config['PREVIEW_CLIENT'] is not None):
+                PREVIEW_CLIENT_CLASS = import_string(app.config['PREVIEW_CLIENT'])
+                PREVIEW_CLIENT_INSTANCE = PREVIEW_CLIENT_CLASS()
+            else:
                 payload = jsonify({'previewData': {}, 'msg': 'A client for the preview feature must be configured'})
                 return make_response(payload, HTTPStatus.NOT_IMPLEMENTED)
 
-        # request table preview data
         response = PREVIEW_CLIENT_INSTANCE.get_preview_data(params=request.get_json())
         status_code = response.status_code
 
@@ -57,7 +60,6 @@ def get_table_preview() -> Response:
             logging.error(message)
             # only necessary to pass the error text
             payload = jsonify({'previewData': {'error_text': preview_data.get('error_text', '')}, 'msg': message})
-
         return make_response(payload, status_code)
     except Exception as e:
         message = 'Encountered exception: ' + str(e)

amundsen_application/config.py

@@ -62,6 +62,16 @@ class Config:
     # Initialize custom routes
     INIT_CUSTOM_ROUTES = None  # type: Callable[[Flask], None]
 
+    # Settings for Preview Client integration
+    PREVIEW_CLIENT_ENABLED = os.getenv('PREVIEW_CLIENT_ENABLED') == 'true'  # type: bool
+    # Maps to a class path and name
+    PREVIEW_CLIENT = os.getenv('PREVIEW_CLIENT', None)  # type: Optional[str]
+
+    # Settings for Announcement Client integration
+    ANNOUNCEMENT_CLIENT_ENABLED = os.getenv('ANNOUNCEMENT_CLIENT_ENABLED') == 'true'  # type: bool
+    # Maps to a class path and name
+    ANNOUNCEMENT_CLIENT = os.getenv('ANNOUNCEMENT_CLIENT', None)  # type: Optional[str]
+
     # Settings for Issue tracker integration
     ISSUE_LABELS = []  # type: List[str]
     ISSUE_TRACKER_API_TOKEN = None  # type: str

requirements.txt

@@ -68,6 +68,9 @@ marshmallow-annotations>=2.4.0,<3.0
 # Upstream url: https://pypi.org/project/responses/
 responses==0.12.1
 
+# Required for announcement client
+SQLAlchemy==1.3.20
+
 # A common package that holds the models deifnition and schemas that are used
 # accross different amundsen repositories.
 amundsen-common==0.5.9

setup.cfg

@@ -3,7 +3,7 @@ format = pylint
 exclude = .svc,CVS,.bzr,.hg,.git,__pycache__,venv,venv3,node_modules
 max-complexity = 10
 max-line-length = 120
-ignore = NONE
+ignore = W503
 
 [pep8]
 max-line-length = 120

tests/unit/api/announcements/__init__.py

+# Copyright Contributors to the Amundsen project.
+# SPDX-License-Identifier: Apache-2.0

tests/unit/api/announcements/test_v0.py

+# Copyright Contributors to the Amundsen project.
+# SPDX-License-Identifier: Apache-2.0
+
+import unittest
+from http import HTTPStatus
+
+from flask import Response
+
+from amundsen_application import create_app
+from amundsen_application.api.announcements import v0
+
+local_app = create_app('amundsen_application.config.TestConfig', 'tests/templates')
+ANNOUNCEMENT_CLIENT_CLASS = ('amundsen_application.base.examples.'
+                             'example_announcement_client.SQLAlchemyAnnouncementClient')
+
+
+class AnnouncementTest(unittest.TestCase):
+
+    def setUp(self) -> None:
+        local_app.config['ANNOUNCEMENT_CLIENT_ENABLED'] = True
+
+    def test_no_client_class(self) -> None:
+        """
+        Test that Not Implemented error is raised when PREVIEW_CLIENT is None
+        :return:
+        """
+        # Reset side effects of other tests to ensure that the results are the
+        # same regardless of execution order
+        v0.ANNOUNCEMENT_CLIENT_CLASS = None
+        v0.ANNOUNCEMENT_CLIENT_INSTANCE = None
+
+        local_app.config['ANNOUNCEMENT_CLIENT'] = None
+        with local_app.test_client() as test:
+            response = test.get('/api/announcements/v0/')
+            self.assertEqual(response.status_code, HTTPStatus.NOT_IMPLEMENTED)
+
+    @unittest.mock.patch(ANNOUNCEMENT_CLIENT_CLASS + '._get_posts')
+    def test_good_client_response(self, mock_get_posts) -> None:
+        """
+        :return:
+        """
+        local_app.config['ANNOUNCEMENT_CLIENT'] = ANNOUNCEMENT_CLIENT_CLASS
+        mock_get_posts.return_value = Response(response='',
+                                               status=HTTPStatus.OK)
+        with local_app.test_client() as test:
+            response = test.get('/api/announcements/v0/')
+            self.assertEqual(response.status_code, HTTPStatus.OK)

tests/unit/api/preview/test_v0.py

+# Copyright Contributors to the Amundsen project.
+# SPDX-License-Identifier: Apache-2.0
+
+import unittest
+import json
+from http import HTTPStatus
+
+from flask import Response
+
+from amundsen_application import create_app
+from amundsen_application.api.preview import v0
+from tests.unit.base.test_superset_preview_client import good_json_data, bad_json_data
+
+local_app = create_app('amundsen_application.config.TestConfig', 'tests/templates')
+PREVIEW_CLIENT_CLASS = 'amundsen_application.base.examples.example_superset_preview_client.SupersetPreviewClient'
+
+
+class PreviewTest(unittest.TestCase):
+
+    def setUp(self) -> None:
+        local_app.config['PREVIEW_CLIENT_ENABLED'] = True
+
+    def test_no_client_class(self) -> None:
+        """
+        Test that Not Implemented error is raised when PREVIEW_CLIENT is None
+        :return:
+        """
+        # Reset side effects of other tests to ensure that the results are the
+        # same regardless of execution order
+        v0.PREVIEW_CLIENT_CLASS = None
+        v0.PREVIEW_CLIENT_INSTANCE = None
+
+        local_app.config['PREVIEW_CLIENT'] = None
+        with local_app.test_client() as test:
+            response = test.post('/api/preview/v0/')
+            self.assertEqual(response.status_code, HTTPStatus.NOT_IMPLEMENTED)
+
+    @unittest.mock.patch(PREVIEW_CLIENT_CLASS + '.get_preview_data')
+    def test_good_client_response(self, mock_get_preview_data) -> None:
+        """
+        """
+        expected_response_json = {
+            'msg': 'Success',
+            'previewData': {
+                'columns': [{}, {}],
+                'data': [{'id': 1, 'name': 'Admin'}, {'id': 2, 'name': 'Public'}, {'id': 3, 'name': 'Alpha'}]}
+        }
+
+        local_app.config['PREVIEW_CLIENT'] = PREVIEW_CLIENT_CLASS
+        response = json.dumps({'preview_data': good_json_data})
+        mock_get_preview_data.return_value = Response(response=response,
+                                                      status=HTTPStatus.OK)
+        with local_app.test_client() as test:
+            response = test.post('/api/preview/v0/')
+            self.assertEqual(response.status_code, HTTPStatus.OK)
+            self.assertEqual(response.json, expected_response_json)
+
+    @unittest.mock.patch(PREVIEW_CLIENT_CLASS + '.get_preview_data')
+    def test_bad_client_response(self, mock_get_preview_data) -> None:
+        """
+        """
+        expected_response_json = {
+            'msg': 'Encountered exception: The preview client did not return a valid PreviewData object',
+            'previewData': {}
+        }
+
+        local_app.config['PREVIEW_CLIENT'] = PREVIEW_CLIENT_CLASS
+        response = json.dumps({'preview_data': bad_json_data})
+        mock_get_preview_data.return_value = Response(response=response,
+                                                      status=HTTPStatus.OK)
+        with local_app.test_client() as test:
+            response = test.post('/api/preview/v0/')
+            self.assertEqual(response.status_code,
+                             HTTPStatus.INTERNAL_SERVER_ERROR)
+            self.assertEqual(response.json, expected_response_json)