-
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
fix(tableau): assert error on parent name #12392
fix(tableau): assert error on parent name #12392
Conversation
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py
Outdated
Show resolved
Hide resolved
metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py
Outdated
Show resolved
Hide resolved
…u.py Co-authored-by: Mayuri Nehate <[email protected]>
metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py
Outdated
Show resolved
Hide resolved
metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py
Outdated
Show resolved
Hide resolved
"project-issue", | ||
f"Parent project {cur_proj.parent_id} not found. We need Site Administrator Explorer permissions.", | ||
) | ||
break |
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.
maybe just add a comment that this indexing is safe based on the guarantees provided by fetch_projects
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.
I added the assert a few lines above with that specific purpose
…u.py Co-authored-by: Harshal Sheth <[email protected]>
…u.py Co-authored-by: Harshal Sheth <[email protected]>
While
Site Administrator Explorer
privileges are required, users occasionally need to operate with a lower level of privileges. If so, the following error is thrown:This occurs because the Tableau API returns partial information when listing projects: child projects might include a reference to the parent project id, but the listing omits those projects that the credentials aren't allowed to access. As a result, the ingestor cannot perform the project name enrichment for such a parent project ID.
In this PR, when this happens, we remove the reference to the parent project from the child. This allows us to provide at least partial but functional information.
Checklist