feedback 2

Remove typing from requirements
Move put methods to a roundtrip test class. Fix super() so this works in roundtrip tests with multiple inheritance.

Signed-off-by: Joshua Hoskins <[email protected]>

metadata_service/proxy/atlas_proxy.py

@@ -997,34 +997,3 @@ def get_resources_using_table(self, *,
                                   id: str,
                                   resource_type: ResourceType) -> Dict[str, List[DashboardSummary]]:
         return {}
-
-    def put_user(self, *, data: User) -> None:
-        pass
-
-    def post_users(self, *, data: List[User]) -> None:
-        pass
-
-    def put_app(self, *, data: Application) -> None:
-        pass
-
-    def post_apps(self, *, data: List[Application]) -> None:
-        pass
-
-    def put_table(self, *, table: Table) -> None:
-        pass
-
-    def post_tables(self, *, tables: List[Table]) -> None:
-        pass
-
-    def put_column(self, *, table_uri: str, column: Column) -> None:
-        pass
-
-    def put_programmatic_table_description(self, *, table_uri: str, description: ProgrammaticDescription) -> None:
-        pass
-
-    def put_programmatic_column_description(self, *, table_uri: str, column_name: str,
-                                            description: Description) -> None:
-        pass
-
-    def add_read_count(self, *, table_uri: str, user_id: str, read_count: int) -> None:
-        pass

metadata_service/proxy/base_proxy.py

@@ -5,7 +5,7 @@
 from typing import Any, Dict, List, Union
 
 from amundsen_common.models.popular_table import PopularTable
-from amundsen_common.models.table import Table, Application, Column, ProgrammaticDescription
+from amundsen_common.models.table import Table
 from amundsen_common.models.user import User
 from amundsen_common.models.dashboard import DashboardSummary
 
@@ -151,42 +151,3 @@ def get_resources_using_table(self, *,
                                   id: str,
                                   resource_type: ResourceType) -> Dict[str, List[DashboardSummary]]:
         pass
-
-    # NB: the following methods exist solely for testing in the roundtrip tests.
-    # These may not be suitably performant for handling large batches
-
-    @abstractmethod
-    def put_user(self, *, data: User) -> None:
-        pass
-
-    @abstractmethod
-    def post_users(self, *, data: List[User]) -> None:
-        pass
-
-    @abstractmethod
-    def put_app(self, *, data: Application) -> None:
-        pass
-
-    @abstractmethod
-    def post_apps(self, *, data: List[Application]) -> None:
-        pass
-
-    @abstractmethod
-    def put_table(self, *, table: Table) -> None:
-        pass
-
-    @abstractmethod
-    def post_tables(self, *, tables: List[Table]) -> None:
-        pass
-
-    @abstractmethod
-    def put_column(self, *, table_uri: str, column: Column) -> None:
-        pass
-
-    @abstractmethod
-    def put_programmatic_table_description(self, *, table_uri: str, description: ProgrammaticDescription) -> None:
-        pass
-
-    @abstractmethod
-    def add_read_count(self, *, table_uri: str, user_id: str, read_count: int) -> None:
-        pass

metadata_service/proxy/janus_graph_proxy.py

@@ -38,8 +38,8 @@ def __init__(self, *, host: str, port: Optional[int] = None, user: Optional[str]
         driver_remote_connection_options.update(traversal_source=traversal_source)
 
         # use _key
-        super().__init__(key_property_name='_key',
-                         driver_remote_connection_options=driver_remote_connection_options)
+        AbstractGremlinProxy.__init__(self, key_property_name='_key',
+                                      driver_remote_connection_options=driver_remote_connection_options)
 
     @classmethod
     @overrides

metadata_service/proxy/neo4j_proxy.py

@@ -1351,30 +1351,3 @@ def get_resources_using_table(self, *,
         for record in records:
             results.append(DashboardSummary(**record))
         return {'dashboards': results}
-
-    def put_user(self, *, data: User) -> None:
-        pass
-
-    def post_users(self, *, data: List[User]) -> None:
-        pass
-
-    def put_app(self, *, data: Application) -> None:
-        pass
-
-    def post_apps(self, *, data: List[Application]) -> None:
-        pass
-
-    def put_table(self, *, table: Table) -> None:
-        pass
-
-    def post_tables(self, *, tables: List[Table]) -> None:
-        pass
-
-    def put_column(self, *, table_uri: str, column: Column) -> None:
-        pass
-
-    def put_programmatic_table_description(self, *, table_uri: str, description: ProgrammaticDescription) -> None:
-        pass
-
-    def add_read_count(self, *, table_uri: str, user_id: str, read_count: int) -> None:
-        pass

metadata_service/proxy/neptune_proxy.py

@@ -3,21 +3,16 @@
 
 import json
 import logging
-from typing import Any, Dict, List, Mapping, Optional, Type, Union
+from typing import Any, Dict, Mapping, Optional, Type, Union
 from urllib.parse import urlsplit, urlunsplit
 
 import boto3
 import gremlin_python.driver.protocol
 import requests
-from amundsen_common.models.table import Table, Application
-from amundsen_common.models.user import User
 from amundsen_gremlin.gremlin_model import WellKnownProperties
 from amundsen_gremlin.neptune_bulk_loader.api import (
     NeptuneBulkLoaderApi, get_neptune_graph_traversal_source_factory
 )
-from amundsen_gremlin.neptune_bulk_loader.gremlin_model_converter import (
-    GetGraph
-)
 from amundsen_gremlin.script_translator import ScriptTranslatorTargetNeptune
 from amundsen_gremlin.test_and_development_shard import get_shard
 from for_requests.assume_role_aws4auth import AssumeRoleAWS4Auth
@@ -122,8 +117,8 @@ def __init__(self, *, host: str, port: Optional[int] = None, user: str = None,
         self.neptune_graph_traversal_source_factory = get_neptune_graph_traversal_source_factory(session=password,
                                                                                                  neptune_url=host)
 
-        super().__init__(key_property_name='key',
-                         driver_remote_connection_options=driver_remote_connection_options)
+        AbstractGremlinProxy.__init__(self, key_property_name='key',
+                                      driver_remote_connection_options=driver_remote_connection_options)
 
     @classmethod
     @overrides
@@ -278,30 +273,3 @@ def drop(self) -> None:
                               get=FromResultSet.iterate)
         assert not leftover, f'we have some leftover: {leftover}'
         LOGGER.warning('COMPLETED DROP OF ALL NODES')
-
-    @overrides
-    def post_users(self, *, data: List[User]) -> None:
-        entities = GetGraph.user_entities(user_data=data, g=self.neptune_graph_traversal_source_factory())
-        self.neptune_bulk_loader_api.bulk_load_entities(entities=entities)
-
-    @overrides
-    def put_user(self, *, data: User) -> None:
-        self.post_users(data=[data])
-
-    @overrides
-    def put_app(self, *, data: Application) -> None:
-        self.post_apps(data=[data])
-
-    @overrides
-    def post_apps(self, *, data: List[Application]) -> None:
-        entities = GetGraph.app_entities(app_data=data, g=self.neptune_graph_traversal_source_factory())
-        self.neptune_bulk_loader_api.bulk_load_entities(entities=entities)
-
-    @overrides
-    def put_table(self, *, table: Table) -> None:
-        self.post_tables(tables=[table])
-
-    @overrides
-    def post_tables(self, *, tables: List[Table]) -> None:
-        entities = GetGraph.table_entities(table_data=tables, g=self.neptune_graph_traversal_source_factory())
-        self.neptune_bulk_loader_api.bulk_load_entities(entities=entities)

requirements.txt

@@ -31,7 +31,6 @@ pytest-mock==1.1
 # PEP 484
 # License: PSF
 # Upstream url: https://github.com/python/typing
-typing==3.6.4
 typing-extensions==3.7.4
 
 # A common package that holds the models deifnition and schemas that are used

tests/unit/proxy/roundtrip/__init__.py

@@ -0,0 +1,2 @@
+# Copyright Contributors to the Amundsen project.
+# SPDX-License-Identifier: Apache-2.0

tests/unit/proxy/abstract_proxy_tests.py → tests/unit/proxy/roundtrip/abstract_proxy_tests.py

@@ -17,14 +17,14 @@
 
 from amundsen_common.models.popular_table import PopularTable
 from metadata_service.entity.tag_detail import TagDetail
-from metadata_service.proxy.base_proxy import BaseProxy
 from metadata_service.proxy.shared import checkNotNone
 from metadata_service.util import UserResourceRel
 from metadata_service.entity.resource_type import ResourceType
+from .roundtrip_base_proxy import RoundtripBaseProxy
 
 __all__ = ['abstract_proxy_test_class']
 
-T = TypeVar('T', bound=BaseProxy)
+T = TypeVar('T', bound=RoundtripBaseProxy)
 
 LOGGER = logging.getLogger(__name__)
 
@@ -68,7 +68,6 @@ def test_rt_table(self) -> None:
         """
         it'd be nice to check that the result could be deserialized as a client of the metadata_service would
         """
-        # TODO: check fixtures
         expected = Fixtures.next_table()
         expected.description = '"hello!" said no one'
         expected.tags.sort()
@@ -80,7 +79,7 @@ def test_rt_table(self) -> None:
 
         self.assertEqual(expected, actual)
 
-    def test_rt_table_with_owner_user_not_application(self) -> None:
+    def test_rt_table_with_owner(self) -> None:
         user = Fixtures.next_user(is_active=True)
         self.get_proxy().put_user(data=user)
         application = Fixtures.next_application(application_id=user.user_id)

tests/unit/proxy/roundtrip/roundtrip_base_proxy.py

@@ -0,0 +1,46 @@
+from metadata_service.proxy import BaseProxy
+from abc import abstractmethod
+from amundsen_common.models.table import Table, Application, Column, ProgrammaticDescription
+from amundsen_common.models.user import User
+from typing import List
+
+
+class RoundtripBaseProxy(BaseProxy):
+    """
+    A base proxy that supports roundtrip tests
+    """
+    @abstractmethod
+    def put_user(self, *, data: User) -> None:
+        pass
+
+    @abstractmethod
+    def post_users(self, *, data: List[User]) -> None:
+        pass
+
+    @abstractmethod
+    def put_app(self, *, data: Application) -> None:
+        pass
+
+    @abstractmethod
+    def post_apps(self, *, data: List[Application]) -> None:
+        pass
+
+    @abstractmethod
+    def put_table(self, *, table: Table) -> None:
+        pass
+
+    @abstractmethod
+    def post_tables(self, *, tables: List[Table]) -> None:
+        pass
+
+    @abstractmethod
+    def put_column(self, *, table_uri: str, column: Column) -> None:
+        pass
+
+    @abstractmethod
+    def put_programmatic_table_description(self, *, table_uri: str, description: ProgrammaticDescription) -> None:
+        pass
+
+    @abstractmethod
+    def add_read_count(self, *, table_uri: str, user_id: str, read_count: int) -> None:
+        pass