feat(ingest): Glue - Support for domains and containers #4110

treff7es · 2022-02-10T08:30:59Z

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable)

Adding option to set aws profile for Glue.

github-actions · 2022-02-10T08:56:44Z

Unit Test Results (metadata ingestion)

    3 files ±  0     3 suites ±0 36m 46s ⏱️ - 6m 16s
317 tests +  5 317 ✔️ +5   0 💤 ±0 0 ❌ ±0
908 runs +12 879 ✔️ +3 29 💤 +9 0 ❌ ±0

Results for commit 580a724. ± Comparison against base commit 9bdc9af.

This pull request removes 1 and adds 6 tests. Note that renamed tests count towards both.

tests.unit.test_utilities ‑ test_groupby_unsorted

tests.integration.kafka.test_kafka_state ‑ test_kafka_ingest_with_stateful
tests.unit.stateful_ingestion.test_kafka_state ‑ test_kafka_common_state
tests.unit.test_athena_source ‑ test_athena_get_table_properties
tests.unit.test_athena_source ‑ test_athena_uri
tests.unit.test_kafka_source.KafkaSourceTest ‑ test_kafka_source_stateful_ingestion_requires_platform_instance
tests.unit.test_kafka_source.KafkaSourceTest ‑ test_kafka_source_workunits_with_platform_instance

♻️ This comment has been updated with latest results.

github-actions · 2022-02-10T08:58:14Z

Unit Test Results (build & test)

  70 files +1   70 suites +1 14m 47s ⏱️ ±0s
609 tests +5 550 ✔️ +5 59 💤 ±0 0 ❌ ±0

Results for commit 580a724. ± Comparison against base commit 9bdc9af.

♻️ This comment has been updated with latest results.

rslanka · 2022-02-11T18:24:10Z

metadata-ingestion/src/datahub/ingestion/source/aws/glue.py

@@ -54,6 +63,7 @@ class GlueSourceConfig(AwsSourceConfig):
    ignore_unsupported_connectors: Optional[bool] = True
    emit_s3_lineage: bool = False
    glue_s3_lineage_direction: str = "upstream"
+    domain: Dict[str, AllowDenyPattern] = dict()


name this domain_pattern?

This is the name in the other sources and I wanted to keep it consistent.

rslanka · 2022-02-11T18:25:03Z

metadata-ingestion/src/datahub/ingestion/source/aws/glue.py

@@ -523,8 +533,66 @@ def get_lineage_if_enabled(
                        return mcp
        return None

-    def get_workunits(self) -> Iterable[MetadataWorkUnit]:
+    def gen_database_key(self, database: str) -> PlatformKey:


Return type annotation looks wrong!
s/PlatformKey/DatabaseKey?

rslanka · 2022-02-11T18:29:55Z

metadata-ingestion/src/datahub/ingestion/source/aws/glue.py

+
+        for domain, pattern in self.source_config.domain.items():
+            if pattern.allowed(dataset_name):
+                domain_urn = make_domain_urn(domain)


return make_domain_urn(domain)

rslanka · 2022-02-11T18:30:06Z

metadata-ingestion/src/datahub/ingestion/source/aws/glue.py

+            if pattern.allowed(dataset_name):
+                domain_urn = make_domain_urn(domain)
+
+        return domain_urn


return None

rslanka · 2022-02-11T18:31:45Z

metadata-ingestion/src/datahub/ingestion/source/aws/glue.py

+
+    def _get_domain_wu(
+        self, dataset_name: str, entity_urn: str, entity_type: str
+    ) -> Iterable[Union[MetadataWorkUnit]]:


Union seems redundant here.

rslanka · 2022-02-11T18:33:22Z

metadata-ingestion/src/datahub/ingestion/source/aws/glue.py

-            flow_names: Dict[str, str] = {}
-
-            for job in self.get_all_jobs():
+    def _transform_extraction(self):


missing return type annotation.

rslanka · 2022-02-11T18:33:39Z

metadata-ingestion/src/datahub/ingestion/source/aws/glue.py

-
-            for job in self.get_all_jobs():
+    def _transform_extraction(self):
+        dags = {}


type annotation?

rslanka · 2022-02-11T18:36:41Z

metadata-ingestion/src/datahub/ingestion/source/aws/glue.py

+        # in Glue, it's possible for two buckets to have files of different extensions
+        # if this happens, we append the extension in the URN so the sources can be distinguished
+        # see process_dataflow_node() for details
+        s3_formats: typing.DefaultDict[str, Set[Union[str, None]]] = defaultdict(


Set[Optional[str]]

rslanka · 2022-02-11T18:42:03Z

metadata-ingestion/src/datahub/ingestion/source/aws/glue.py

+                self.report.report_workunit(wu)
+                yield wu
+
+    def get_workunits(self) -> Iterable[MetadataWorkUnit]:


This has become a 100 line function now. Refactor into smaller functions?

rslanka · 2022-02-11T18:45:13Z

metadata-ingestion/tests/unit/glue/glue_mces_golden.json

@@ -1,4 +1,43 @@
 [
+  {


Have the test coverage numbers remained the same or dropped after this change?

It remained the same==86%.

rslanka · 2022-02-15T20:55:43Z

metadata-ingestion/src/datahub/ingestion/source/aws/aws_common.py

@@ -89,7 +90,7 @@ def get_session(self) -> Session:
                region_name=self.aws_region,
            )
        else:
-            return Session(region_name=self.aws_region)
+            return Session(region_name=self.aws_region, profile_name=self.aws_profile)


Is the profile_name param supported by our min version of boto3?

it is there for 7 years, so I think we should be fine

rslanka · 2022-02-15T20:57:34Z

metadata-ingestion/src/datahub/ingestion/source/aws/glue.py

+
+    def gen_database_containers(self, database: str) -> Iterable[MetadataWorkUnit]:
+        domain_urn = self._gen_domain_urn(database)
+


nit: delete empty line.

rslanka

LGTM!

shirshanka

LGTM

…ect#4110) * Add container and domain support for Glue. Adding option to set aws profile for Glue. * Adding domain doc for Glue * Making get_workunits less complex * Updating golden file * Addressing pr review comments * Remove unneded empty line

treff7es added 2 commits February 10, 2022 09:27

Add container and domain support for Glue.

28a07d3

Adding option to set aws profile for Glue.

Adding domain doc for Glue

5cd62ef

treff7es added 2 commits February 10, 2022 10:34

Making get_workunits less complex

26666d8

Updating golden file

1adb782

rslanka reviewed Feb 11, 2022

View reviewed changes

Addressing pr review comments

ca1503c

rslanka reviewed Feb 15, 2022

View reviewed changes

rslanka approved these changes Feb 15, 2022

View reviewed changes

Remove unneded empty line

580a724

shirshanka approved these changes Feb 16, 2022

View reviewed changes

shirshanka merged commit b266491 into datahub-project:master Feb 16, 2022

treff7es deleted the glue_domains_containers branch February 8, 2023 11:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ingest): Glue - Support for domains and containers #4110

feat(ingest): Glue - Support for domains and containers #4110

treff7es commented Feb 10, 2022

github-actions bot commented Feb 10, 2022 •

edited

Loading

github-actions bot commented Feb 10, 2022 •

edited

Loading

rslanka Feb 11, 2022

treff7es Feb 14, 2022

rslanka Feb 11, 2022

rslanka Feb 11, 2022

rslanka Feb 11, 2022 •

edited

Loading

rslanka Feb 11, 2022

rslanka Feb 11, 2022

rslanka Feb 11, 2022

rslanka Feb 11, 2022

rslanka Feb 11, 2022

rslanka Feb 11, 2022

treff7es Feb 14, 2022

rslanka Feb 15, 2022

treff7es Feb 16, 2022

rslanka Feb 15, 2022 •

edited

Loading

rslanka left a comment

shirshanka left a comment


		def gen_database_containers(self, database: str) -> Iterable[MetadataWorkUnit]:
		domain_urn = self._gen_domain_urn(database)

feat(ingest): Glue - Support for domains and containers #4110

feat(ingest): Glue - Support for domains and containers #4110

Conversation

treff7es commented Feb 10, 2022

Checklist

github-actions bot commented Feb 10, 2022 • edited Loading

Unit Test Results (metadata ingestion)

github-actions bot commented Feb 10, 2022 • edited Loading

Unit Test Results (build & test)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rslanka Feb 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rslanka Feb 15, 2022 • edited Loading

Choose a reason for hiding this comment

rslanka left a comment

Choose a reason for hiding this comment

shirshanka left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 10, 2022 •

edited

Loading

github-actions bot commented Feb 10, 2022 •

edited

Loading

rslanka Feb 11, 2022 •

edited

Loading

rslanka Feb 15, 2022 •

edited

Loading