Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(ingest/tableau): Fetch Upstreams From Columns #9874

Merged
merged 30 commits into from
May 8, 2024
Merged

feat(ingest/tableau): Fetch Upstreams From Columns #9874

merged 30 commits into from
May 8, 2024

Conversation

egemenberk
Copy link
Contributor

@egemenberk egemenberk commented Feb 16, 2024 •
edited
Loading

This PR fixes the following two things:

  • Tableau metadata graphql API returns empty list for upstreamTables for embedded datasources while upstreamColumns field includes information. This PR populates upstream table information from upstreamColumns field coming from the Tableau
  • Tableau metadata graphql API returns malfunctioned SQL queries which causing failures in fetching upstream Lineage from the CustomSQLs. This also enables embedded data sources to be connected to the CustomSQLs which are (generally) connected to upstreams from other platforms hence completing the full lineage.

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

@egemenberk
fix(ingestion): Fetch Upstreams From Columns
7f39413 
Tableau API returns empty list for upstreamTables
Construct the upstreamLineage from upstreamColumns field
Connect CustomSQLs to corresponding embedded data sources
@egemenberk
Merge branch 'datahub-project:master' into fix/create-lineage-between…
24ceca9 
…-custom-sqls-and-embedded-data-sources
@egemenberk
Clean Tableau Query
a2abd4b
@egemenberk
Merge branch 'fix/create-lineage-between-custom-sqls-and-embedded-dat…
30baa84 
…a-sources' of https://github.com/egemenberk/datahub into fix/create-lineage-between-custom-sqls-and-embedded-data-sources
@github-actions github-actions bot added ingestion PR or Issue related to the ingestion of metadata community-contribution PR or Issue raised by member(s) of DataHub Community labels Feb 16, 2024
@egemenberk egemenberk changed the title Fix/create lineage between custom sqls and embedded data sources fix(ingestion): Fetch Upstreams From Columns Feb 16, 2024
@asikowitz asikowitz self-assigned this Feb 22, 2024
@asikowitz asikowitz self-requested a review February 22, 2024 18:48
asikowitz
asikowitz previously requested changes Feb 23, 2024
View reviewed changes
Copy link
Collaborator

@asikowitz asikowitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! I have a couple style requests. Can you also update tests/integration/tableau/setup/embeddedDatasourcesConnection_all.json to add a data source that has no upstream tables, but still has lineage information in upstream columns, to test this new functionality?

metadata-ingestion/src/datahub/ingestion/source/tableau.py Outdated Show resolved Hide resolved
metadata-ingestion/src/datahub/ingestion/source/tableau.py Outdated Show resolved Hide resolved
metadata-ingestion/src/datahub/ingestion/source/tableau.py Outdated Show resolved Hide resolved
@egemenberk egemenberk requested a review from asikowitz March 14, 2024 14:09
@hsheth2
Copy link
Collaborator

hsheth2 commented Mar 18, 2024

@egemenberk some query cleaning logic is also getting adding in this PR #9838 - that one also removes parameter names and things to make SQL parsing work. Does it make sense to unify across these two query cleaning implementations?

@egemenberk
Copy link
Contributor Author

@egemenberk some query cleaning logic is also getting adding in this PR #9838 - that one also removes parameter names and things to make SQL parsing work. Does it make sense to unify across these two query cleaning implementations?

Hi @hsheth2, I've taken a quick look at the PR you mentioned and it seems to fix the query, so I can remove the clean_query() method call from my implementation. My PR's main focus is to fix fetching upstream lineage from upstreamColumns when upstreamTables field is empty in Tableau response, the clean_query addition was a side fix while working on the task, so I can remove my addition on that and trust the #9838 implementation. Thanks for the information 👍

@egemenberk egemenberk mentioned this pull request Mar 20, 2024
Merged
5 tasks
@egemenberk egemenberk requested a review from hsheth2 April 30, 2024 13:20
@hsheth2 hsheth2 changed the title fix(ingestion): Fetch Upstreams From Columns feat(ingest/tableau): Fetch Upstreams From Columns May 8, 2024
@hsheth2 hsheth2 dismissed asikowitz’s stale review May 8, 2024 21:21

Stale review

@hsheth2 hsheth2 merged commit b1b7ced into datahub-project:master May 8, 2024
57 checks passed
sleeperdeep pushed a commit to sleeperdeep/datahub that referenced this pull request Jun 25, 2024
@egemenberk @sleeperdeep
feat(ingest/tableau): Fetch Upstreams From Columns (datahub-project#9874
e56d385 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hsheth2 hsheth2 hsheth2 approved these changes

@asikowitz asikowitz Awaiting requested review from asikowitz

Assignees

@asikowitz asikowitz

@hsheth2 hsheth2

Labels
community-contribution PR or Issue raised by member(s) of DataHub Community ingestion PR or Issue related to the ingestion of metadata
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@egemenberk @hsheth2 @asikowitz