fix: Upgrade mypy version to build with Python3.8 (#975)

* Upgraded mypy version and fixed issues introduced in change

Signed-off-by: Grant Seward <grant@stemma.ai>

* Added python3.8 to test matrix

Signed-off-by: Grant Seward <grant@stemma.ai>

.github/workflows/pull_request.yml

@@ -16,7 +16,7 @@ jobs:
     runs-on: ubuntu-18.04
     strategy:
       matrix:
-        python-version: ["3.6.x", "3.7.x"]
+        python-version: ["3.6.x", "3.7.x", "3.8.x"]
     steps:
       - name: Checkout
         uses: actions/checkout@v1

amundsen_application/__init__.py

@@ -29,7 +29,7 @@
     moduleName = os.getenv('APP_WRAPPER', '')
     module = importlib.import_module(moduleName)
     moduleClass = os.getenv('APP_WRAPPER_CLASS', '')
-    app_wrapper_class = getattr(module, moduleClass)
+    app_wrapper_class = getattr(module, moduleClass)  # type: ignore
 
 PROJECT_ROOT = os.path.dirname(os.path.abspath(__file__))
 STATIC_ROOT = os.getenv('STATIC_ROOT', 'static')
@@ -50,10 +50,10 @@ def create_app(config_module_class: str = None, template_folder: str = None) ->
     app.config.from_object(config_module_class)
 
     if app.config.get('LOG_CONFIG_FILE'):
-        logging.config.fileConfig(app.config.get('LOG_CONFIG_FILE'), disable_existing_loggers=False)
+        logging.config.fileConfig(app.config['LOG_CONFIG_FILE'], disable_existing_loggers=False)
     else:
-        logging.basicConfig(format=app.config.get('LOG_FORMAT'), datefmt=app.config.get('LOG_DATE_FORMAT'))
-        logging.getLogger().setLevel(app.config.get('LOG_LEVEL'))
+        logging.basicConfig(format=app.config['LOG_FORMAT'], datefmt=app.config.get('LOG_DATE_FORMAT'))
+        logging.getLogger().setLevel(app.config['LOG_LEVEL'])
 
     logging.info('Created app with config name {}'.format(config_module_class))
     logging.info('Using metadata service at {}'.format(app.config.get('METADATASERVICE_BASE')))

amundsen_application/base/base_mail_client.py

@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import abc
-from typing import Dict, List
+from typing import Dict, List, Optional
 
 from flask import Response
 
@@ -16,16 +16,19 @@ def __init__(self, recipients: List[str]) -> None:
     def send_email(self,
                    html: str,
                    subject: str,
-                   optional_data: Dict,
-                   recipients: List[str],
-                   sender: str) -> Response:
+                   optional_data: Optional[Dict] = None,
+                   recipients: Optional[List[str]] = None,
+                   sender: Optional[str] = None) -> Response:
         """
         Sends an email using the following parameters
         :param html: HTML email content
         :param subject: The subject of the email
-        :param optional_data: A dictionary of any values needed for custom implementations
-        :param recipients: A list of recipients for the email
-        :param sender: The sending address associated with the email
+        :param optional_data: An optional dictionary of any values needed for custom implementations
+        :param recipients: An optional list of recipients for the email, the implementation
+            for this class should determine whether to use the recipients from the function,
+            the __init__ or both
+        :param sender: An optional sending address associated with the email, the implementation
+            should determine whether to use this value or another (e.g. from envvars)
         :return:
         """
         raise NotImplementedError  # pragma: no cover

amundsen_application/tests/test_utils.py

@@ -7,7 +7,7 @@
 TEST_USER_ID = 'test_user_id'
 
 
-def get_test_user(app: app) -> User:
+def get_test_user(app: app) -> User:  # type: ignore
     user_info = {
         'email': 'test@email.com',
         'user_id': TEST_USER_ID,

requirements.txt

@@ -31,7 +31,7 @@ pytest-mock==3.5.1
 # Mypy is an optional static type checker for Python.
 # License: MIT
 # Upstream url: https://github.com/python/mypy
-mypy==0.660
+mypy==0.812
 
 # PEP 484
 # License: PSF

tests/unit/api/announcements/test_v0.py

@@ -35,7 +35,7 @@ def test_no_client_class(self) -> None:
             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:
+    def test_good_client_response(self, mock_get_posts: unittest.mock.Mock) -> None:
         """
         :return:
         """

tests/unit/api/issue/test_issue.py

@@ -49,7 +49,7 @@ def test_get_issues_not_enabled(self) -> None:
             self.assertEqual(response.status_code, HTTPStatus.ACCEPTED)
 
     @unittest.mock.patch('amundsen_application.api.issue.issue.get_issue_tracker_client')
-    def test_get_jira_issues_missing_config(self, mock_issue_tracker_client) -> None:
+    def test_get_jira_issues_missing_config(self, mock_issue_tracker_client: unittest.mock.Mock) -> None:
         """
         Test request failure if config settings are missing
         :return:
@@ -70,7 +70,7 @@ def test_get_jira_issues_no_key(self) -> None:
             self.assertEqual(response.status_code, HTTPStatus.INTERNAL_SERVER_ERROR)
 
     @unittest.mock.patch('amundsen_application.api.issue.issue.get_issue_tracker_client')
-    def test_get_jira_issues_success(self, mock_issue_tracker_client) -> None:
+    def test_get_jira_issues_success(self, mock_issue_tracker_client: unittest.mock.Mock) -> None:
         """
         Tests successful get request
         :return:
@@ -104,7 +104,7 @@ def test_create_issue_not_enabled(self) -> None:
             self.assertEqual(response.status_code, HTTPStatus.ACCEPTED)
 
     @unittest.mock.patch('amundsen_application.api.issue.issue.get_issue_tracker_client')
-    def test_create_jira_issue_missing_config(self, mock_issue_tracker_client) -> None:
+    def test_create_jira_issue_missing_config(self, mock_issue_tracker_client: unittest.mock.Mock) -> None:
         """
         Test request failure if config settings are missing
         :return:
@@ -156,7 +156,7 @@ def test_create_jira_issue_no_title(self) -> None:
             self.assertEqual(response.status_code, HTTPStatus.INTERNAL_SERVER_ERROR)
 
     @unittest.mock.patch('amundsen_application.api.issue.issue.get_issue_tracker_client')
-    def test_create_jira_issue_success(self, mock_issue_tracker_client) -> None:
+    def test_create_jira_issue_success(self, mock_issue_tracker_client: unittest.mock.Mock) -> None:
         """
         Test request returns success and expected outcome
         :return:

tests/unit/api/mail/test_v0.py

@@ -2,9 +2,10 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import unittest
+from unittest.mock import Mock
 
 from http import HTTPStatus
-from typing import Dict, List
+from typing import Dict, List, Optional
 
 from flask import Response, jsonify, make_response
 
@@ -19,12 +20,11 @@ def __init__(self, status_code: int, recipients: List = []) -> None:
         self.status_code = status_code
 
     def send_email(self,
-                   sender: str = None,
-                   recipients: List = [],
-                   subject: str = None,
-                   text: str = None,
-                   html: str = None,
-                   optional_data: Dict = {}) -> Response:
+                   html: str,
+                   subject: str,
+                   optional_data: Optional[Dict] = None,
+                   recipients: Optional[List[str]] = None,
+                   sender: Optional[str] = None) -> Response:
         return make_response(jsonify({}), self.status_code)
 
 
@@ -33,12 +33,11 @@ def __init__(self) -> None:
         pass
 
     def send_email(self,
-                   sender: str = None,
-                   recipients: List = [],
-                   subject: str = None,
-                   text: str = None,
-                   html: str = None,
-                   optional_data: Dict = {}) -> Response:
+                   html: str,
+                   subject: str,
+                   optional_data: Optional[Dict] = None,
+                   recipients: Optional[List[str]] = None,
+                   sender: Optional[str] = None) -> Response:
         raise Exception('Bad client')
 
 
@@ -96,15 +95,15 @@ def test_feedback_client_propagate_status_code(self) -> None:
             self.assertEqual(response.status_code, expected_code)
 
     @unittest.mock.patch('amundsen_application.api.mail.v0.send_notification')
-    def test_notification_endpoint_calls_send_notification(self, send_notification_mock) -> None:
+    def test_notification_endpoint_calls_send_notification(self, send_notification_mock: Mock) -> None:
         """
         Test that the endpoint calls send_notification with the correct information
         from the request json
         :return:
         """
         test_recipients = ['test@test.com']
         test_notification_type = 'added'
-        test_options = {}
+        test_options: Dict = {}
 
         with local_app.test_client() as test:
             test.post('/api/mail/v0/notification', json={
@@ -120,15 +119,15 @@ def test_notification_endpoint_calls_send_notification(self, send_notification_m
             )
 
     @unittest.mock.patch('amundsen_application.api.mail.v0.send_notification')
-    def test_notification_endpoint_fails_missing_notification_type(self, send_notification_mock) -> None:
+    def test_notification_endpoint_fails_missing_notification_type(self, send_notification_mock: Mock) -> None:
         """
         Test that the endpoint fails if notificationType is not provided in the
         request json
         :return:
         """
         test_recipients = ['test@test.com']
         test_sender = 'test2@test.com'
-        test_options = {}
+        test_options: Dict = {}
 
         with local_app.test_client() as test:
             response = test.post('/api/mail/v0/notification', json={
@@ -140,7 +139,7 @@ def test_notification_endpoint_fails_missing_notification_type(self, send_notifi
             self.assertFalse(send_notification_mock.called)
 
     @unittest.mock.patch('amundsen_application.api.mail.v0.send_notification')
-    def test_notification_endpoint_fails_with_exception(self, send_notification_mock) -> None:
+    def test_notification_endpoint_fails_with_exception(self, send_notification_mock: Mock) -> None:
         """
         Test that the endpoint returns 500 exception when error occurs
         and that send_notification is not called

tests/unit/api/metadata/test_v0.py

@@ -1021,7 +1021,7 @@ def test_get_user_read(self) -> None:
             response = test.get('/api/metadata/v0/user/read', query_string=dict(user_id=test_user))
             data = json.loads(response.data)
             self.assertEqual(response.status_code, HTTPStatus.OK)
-            self.assertCountEqual(data.get('read'), self.expected_parsed_user_resources.get('table'))
+            self.assertCountEqual(data.get('read'), self.expected_parsed_user_resources.get('table'))  # type: ignore
 
     @responses.activate
     def test_get_user_own(self) -> None:
@@ -1093,11 +1093,10 @@ def test_get_related_dashboards_success(self) -> None:
                 '/api/metadata/v0/table/{0}/dashboards'.format(test_table)
             )
             data = json.loads(response.data)
-
             self.assertEqual(response.status_code, HTTPStatus.OK)
             self.assertEqual(
                 len(data.get('dashboards')),
-                len(self.expected_related_dashboard_response.get('dashboards'))
+                len(self.expected_related_dashboard_response['dashboards'])
             )
 
     @responses.activate

tests/unit/api/preview/test_v0.py

@@ -36,7 +36,7 @@ def test_no_client_class(self) -> None:
             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:
+    def test_good_client_response(self, mock_get_preview_data: unittest.mock.Mock) -> None:
         """
         """
         expected_response_json = {
@@ -51,12 +51,12 @@ def test_good_client_response(self, mock_get_preview_data) -> None:
         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)
+            post_response = test.post('/api/preview/v0/')
+            self.assertEqual(post_response.status_code, HTTPStatus.OK)
+            self.assertEqual(post_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:
+    def test_bad_client_response(self, mock_get_preview_data: unittest.mock.Mock) -> None:
         """
         """
         expected_response_json = {
@@ -69,7 +69,7 @@ def test_bad_client_response(self, mock_get_preview_data) -> None:
         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,
+            post_response = test.post('/api/preview/v0/')
+            self.assertEqual(post_response.status_code,
                              HTTPStatus.INTERNAL_SERVER_ERROR)
-            self.assertEqual(response.json, expected_response_json)
+            self.assertEqual(post_response.json, expected_response_json)

tests/unit/base/test_issue_tracker_client.py

@@ -6,32 +6,19 @@
 
 from amundsen_application.base.base_issue_tracker_client import BaseIssueTrackerClient
 
+
 app = flask.Flask(__name__)
 app.config.from_object('amundsen_application.config.TestConfig')
 
 
 class IssueTrackerClientTest(unittest.TestCase):
-    def setUp(self) -> None:
-        BaseIssueTrackerClient.__abstractmethods__ = frozenset()
-        self.client = BaseIssueTrackerClient()
-
-    def tearDown(self) -> None:
-        pass
-
-    def test_cover_get_issues(self) -> None:
-        with app.test_request_context():
-            try:
-                self.client.get_issues(table_uri='test')
-            except NotImplementedError:
-                self.assertTrue(True)
-            else:
-                self.assertTrue(False)
 
-    def test_cover_create_issue(self) -> None:
-        with app.test_request_context():
-            try:
-                self.client.create_issue(table_uri='test', title='title', description='description')
-            except NotImplementedError:
-                self.assertTrue(True)
-            else:
-                self.assertTrue(False)
+    def test_abstract_class_methods(self) -> None:
+        abstract_methods_set = {
+            '__init__',
+            'create_issue',
+            'get_issues'
+        }
+        # Use getattr to prevent mypy warning
+        cls_abstrct_methods = getattr(BaseIssueTrackerClient, '__abstractmethods__')
+        self.assertEquals(cls_abstrct_methods, abstract_methods_set)

tests/unit/base/test_preview_client.py

@@ -11,18 +11,12 @@
 
 
 class PreviewClientTest(unittest.TestCase):
-    def setUp(self) -> None:
-        BasePreviewClient.__abstractmethods__ = frozenset()
-        self.client = BasePreviewClient()
 
-    def tearDown(self) -> None:
-        pass
-
-    def cover_abtract_methods(self) -> None:
-        with app.test_request_context():
-            try:
-                self.client.get_preview_data()
-            except NotImplementedError:
-                self.assertTrue(True)
-            else:
-                self.assertTrue(False)
+    def test_cover_abtract_methods(self) -> None:
+        abstract_methods_set = {
+            '__init__',
+            'get_preview_data'
+        }
+        # Use getattr to prevent mypy warning
+        cls_abstrct_methods = getattr(BasePreviewClient, '__abstractmethods__')
+        self.assertEquals(cls_abstrct_methods, abstract_methods_set)

tests/unit/utils/test_metadata_utils.py

@@ -1,6 +1,7 @@
 # Copyright Contributors to the Amundsen project.
 # SPDX-License-Identifier: Apache-2.0
 
+from typing import Dict
 import unittest
 
 from amundsen_application.api.utils.metadata_utils import _convert_prog_descriptions, _sort_prog_descriptions, \
@@ -95,8 +96,8 @@ def test_convert_prog_descriptions_without_config(self) -> None:
             local_app.config['PROGRAMMATIC_DISPLAY'] = test_config
 
             result = _convert_prog_descriptions(test_desc)
-            self.assertEqual(len(result.get('left')), 0)
-            self.assertEqual(len(result.get('right')), 0)
+            self.assertEqual(len(result['left']), 0)
+            self.assertEqual(len(result['right']), 0)
             self.assertEqual(result.get('other'), expected_programmatic_desc.get('other'))
 
     def test_sort_prog_descriptions_returns_default_value(self) -> None:
@@ -151,7 +152,7 @@ def setUp(self) -> None:
         pass
 
     def test_empty_allowed(self) -> None:
-        mockConfig = {
+        mockConfig: Dict = {
             'UNEDITABLE_SCHEMAS': [],
             'UNEDITABLE_TABLE_DESCRIPTION_MATCH_RULES': [],
         }