-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
feat(ingestion/tableau): hidden asset handling #11559
feat(ingestion/tableau): hidden asset handling #11559
Conversation
# Add hidden tags if sheet is hidden (blank luid) | ||
tags.extend(self.config.tags_for_hidden_assets) | ||
|
||
chart_snapshot.aspects.append( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seemed to be a general issue with the tag ingestion. If all tags are removed in Tableau they wouldn't get removed in Datahub because tags are only saved if there are some tags present. Now, we basically always save the GlobalTags aspect even if there are no tags. But this allows us to properly reset the tags if they were removed from Tableau. I refactored this for all the assets with tags.
This resulted in quite a big changeset in the golden test files. Let me know if you see an issue with this. Thanks.
metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py
Outdated
Show resolved
Hide resolved
Hi @hsheth2, would be great to get your feedback on this. Many thanks. |
metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py
Outdated
Show resolved
Hide resolved
…ure/tableau-ingestion-hidden-views
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly good, requested minor change to avoid breaking change to default behaviour.
…/reset tags if ingest_tags enabled
metadata-ingestion/tests/integration/tableau/test_tableau_ingest.py
Outdated
Show resolved
Hide resolved
…ub.com/haeniya/datahub into feature/tableau-ingestion-hidden-views
…ure/tableau-ingestion-hidden-views
@mayurinehate and @hsheth2 anything missing here? |
Merging the PR now. |
Thank you a lot 🙏 @haeniya @mayurinehate |
Description
This PR enables the configuration of how to handle hidden assets in Tableau. Views and Dashboards can be hidden within Workbooks. Hidden assets can be identified by a missing/blank luid, as stated in the documentation: https://help.tableau.com/current/api/metadata_api/en-us/reference/view.doc.html
This PR introduces two new config properties
tags_for_hidden_assets
andingest_hidden_assets
to better control what to do with hidden assets. They can either be skipped completely or tagged.Configuration in the recipe could look like this:
Closes #6421
Checklist