feat(ingestion): Adding config option to auto lowercase dataset urns #8928

treff7es · 2023-10-02T09:59:31Z

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). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

pedro93 · 2023-10-02T15:46:19Z

metadata-ingestion/src/datahub/ingestion/run/pipeline_config.py

@@ -57,6 +57,11 @@ class FlagsConfig(ConfigModel):
        ),
    )

+    auto_lowercase_urns: bool = Field(
+        default=False,
+        description="Wether to lowercase dataset entity urns.",


Typo: Wether -> whether

metadata-ingestion/src/datahub/ingestion/run/pipeline_config.py

hsheth2 · 2023-10-02T17:26:34Z

metadata-ingestion/src/datahub/ingestion/api/source_helpers.py

+            urn = Urn.create_from_string(wu.get_urn())
+            if urn.get_type() == DatasetUrn.ENTITY_TYPE:
+                dataset_urn = DatasetUrn.create_from_string(str(urn))
+                lowercased_urn = DatasetUrn.create_from_ids(


this only fixes the urn, but won't fix lineage edges or things

Let's use the lowercase_dataset_urns helper method instead

datahub/metadata-ingestion/src/datahub/utilities/urns/urn_iter.py

Line 123 in cbd8e14

def lowercase_dataset_urns(model: DictWrapper) -> None:

vercel · 2023-10-02T22:13:24Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs-website	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 3, 2023 7:32am

hsheth2

The usage of lowercase_dataset_urns looks good, but we do want to move the config to be per-source

metadata-ingestion/src/datahub/ingestion/api/source.py

hsheth2 · 2023-10-03T23:40:45Z

metadata-ingestion/src/datahub/ingestion/api/source_helpers.py

+            yield wu
+        except Exception:
+            logger.warning(
+                f"Failed to lowercase urns for {wu} the exception was: {traceback.format_exc()}"


Suggested change

f"Failed to lowercase urns for {wu} the exception was: {traceback.format_exc()}"

f"Failed to lowercase urns for {wu}: {e}", exc_info=True

hsheth2 · 2023-10-03T23:41:13Z

metadata-ingestion/src/datahub/ingestion/run/pipeline_config.py

@@ -57,6 +57,11 @@ class FlagsConfig(ConfigModel):
        ),
    )

+    auto_lowercase_urns: bool = Field(
+        default=False,
+        description="Whether to lowercase dataset entity urns.",


remove this

hsheth2 · 2023-10-03T23:42:41Z

metadata-ingestion/src/datahub/utilities/urns/urn_iter.py

@@ -32,7 +36,7 @@ def list_urns_with_path(

    if isinstance(model, MetadataChangeProposalWrapper):
        if model.entityUrn:
-            urns.append((model.entityUrn, ["urn"]))
+            urns.append((model.entityUrn, ["entityUrn"]))


pedro93 · 2023-10-04T09:30:07Z

metadata-ingestion/src/datahub/configuration/source_common.py

+class LowerCaseDatasetUrnConfigMixin(ConfigModel):
+    convert_urns_to_lowercase: bool = Field(
+        default=False,
+        description="Whether to convert dataset urns to lowercase.",


Question: This property isn't specific to just dataset urns, is it?
We should ideally have a consistent property across all sources that lower cases any urns it generates or references to ensure we have clean lineage across all sources.

currently, it is only lowercase dataset urns

Is that a design decision? What is the reasoning behind it?

hsheth2 · 2023-10-04T21:33:13Z

@treff7es looks like CI is failing here

treff7es · 2023-10-05T13:22:27Z

@treff7es looks like CI is failing here

shoot, sorry, I fixed it and now it is green

hsheth2

CI is green, but there's a merge conflict now :(

hsheth2 · 2023-10-05T17:59:42Z

metadata-ingestion/src/datahub/ingestion/api/source.py

            and self.ctx.pipeline_config.source.config
+            and hasattr(


now that I'm looking at this again, it's actually kinda tricky - pipeline_config.source.config could be either the pydantic config object or the raw config dict - we need to handle both cases here, so we might need to restore that .get(...) logic you had earlier

now, I handle both case

Adding flag to auto lowercase dataset urns

6b2d222

github-actions bot added the ingestion PR or Issue related to the ingestion of metadata label Oct 2, 2023

vercel bot deployed to Preview October 2, 2023 10:16 View deployment

Fixing linter issues

70de905

vercel bot deployed to Preview October 2, 2023 11:08 View deployment

Fixing typo

2727ee7

vercel bot deployed to Preview October 2, 2023 12:06 View deployment

pedro93 reviewed Oct 2, 2023

View reviewed changes

pedro93 requested a review from hsheth2 October 2, 2023 15:49

pedro93 reviewed Oct 2, 2023

View reviewed changes

metadata-ingestion/src/datahub/ingestion/run/pipeline_config.py Outdated Show resolved Hide resolved

hsheth2 reviewed Oct 2, 2023

View reviewed changes

Using urn iter's lowercase_dataset_urns

8980153

Fixing typo

241a676

vercel bot deployed to Preview October 2, 2023 22:30 View deployment

hsheth2 reviewed Oct 2, 2023

View reviewed changes

Adding config option to lowercase dataset urns

461e8d8

vercel bot deployed to Preview October 3, 2023 07:32 View deployment

Fixing tests

c52ea07

treff7es changed the title ~~feat(ingestion): Adding flag to auto lowercase dataset urns~~ feat(ingestion): Adding config option to auto lowercase dataset urns Oct 3, 2023

vercel bot deployed to Preview October 3, 2023 22:15 View deployment

Fixing tests

e507f60

treff7es requested a review from pedro93 October 3, 2023 22:29

vercel bot deployed to Preview October 3, 2023 23:27 View deployment

hsheth2 reviewed Oct 3, 2023

View reviewed changes

Fixing pr review comments

672c4e4

treff7es requested a review from hsheth2 October 4, 2023 07:17

vercel bot deployed to Preview October 4, 2023 07:20 View deployment

pedro93 reviewed Oct 4, 2023

View reviewed changes

Fixing config check

624f956

vercel bot deployed to Preview October 4, 2023 11:01 View deployment

fixing tests

eb4346e

vercel bot deployed to Preview October 5, 2023 06:12 View deployment

hsheth2 reviewed Oct 5, 2023

View reviewed changes

Merge branch 'master' into auto_lowercase_dataset_urns

669ba6a

vercel bot had a problem deploying to Preview October 6, 2023 07:44 Failure

Fixing to work with Pydantic config and dict as well

b731344

treff7es requested review from hsheth2 and pedro93 October 6, 2023 08:16

vercel bot deployed to Preview October 6, 2023 08:37 View deployment

hsheth2 approved these changes Oct 6, 2023

View reviewed changes

treff7es added 2 commits October 12, 2023 12:13

Merge branch 'master' into auto_lowercase_dataset_urns

5c3c7f4

Merge branch 'master' into auto_lowercase_dataset_urns

9052d94

vercel bot deployed to Preview October 12, 2023 10:50 View deployment

treff7es merged commit c381806 into datahub-project:master Oct 12, 2023

maggiehays added the hacktoberfest-accepted Acceptance for hacktoberfest https://hacktoberfest.com/participation/ label Oct 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ingestion): Adding config option to auto lowercase dataset urns #8928

feat(ingestion): Adding config option to auto lowercase dataset urns #8928

treff7es commented Oct 2, 2023

pedro93 Oct 2, 2023

hsheth2 Oct 2, 2023

vercel bot commented Oct 2, 2023 •

edited

Loading

hsheth2 left a comment •

edited

Loading

hsheth2 Oct 3, 2023

hsheth2 Oct 3, 2023

hsheth2 Oct 3, 2023

pedro93 Oct 4, 2023 •

edited

Loading

treff7es Oct 4, 2023

pedro93 Oct 4, 2023

hsheth2 commented Oct 4, 2023

treff7es commented Oct 5, 2023

hsheth2 left a comment

hsheth2 Oct 5, 2023

treff7es Oct 6, 2023

	f"Failed to lowercase urns for {wu} the exception was: {traceback.format_exc()}"
	f"Failed to lowercase urns for {wu}: {e}", exc_info=True

feat(ingestion): Adding config option to auto lowercase dataset urns #8928

feat(ingestion): Adding config option to auto lowercase dataset urns #8928

Conversation

treff7es commented Oct 2, 2023

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Oct 2, 2023 • edited Loading

hsheth2 left a comment • 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

pedro93 Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hsheth2 commented Oct 4, 2023

treff7es commented Oct 5, 2023

hsheth2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Oct 2, 2023 •

edited

Loading

hsheth2 left a comment •

edited

Loading

pedro93 Oct 4, 2023 •

edited

Loading