feat(ingestion/powerbi): ingest powerbi app #11629
Conversation
sid-acryl
changed the title
github-actions
bot
added
the
ingestion
|
Strange error not strictly linked to my PR, I suppose: |
metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py
Outdated
Show resolved
Hide resolved
It will be fixed now |
…datahub-fork into ing-732-app-ingestion
metadata-ingestion/src/datahub/ingestion/source/powerbi/config.py
Outdated
Show resolved
Hide resolved
| title="App Ingestion Is Disabled", | ||
| message="You are missing workspace app metadata. Please set flag `extract_app` to `true` in recipe to ingest workspace app.", | ||
| context=f"workspace-name={workspace.name}, app-name = {workspace.app.name}", | ||
| ) |
There was a problem hiding this comment.
do we need a return here? when would .app be populated if app ingestion is disabled?
There was a problem hiding this comment.
Added return. .app will be populated on next run after setting extract_app to true
metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi.py
Outdated
Show resolved
Hide resolved
| ] | ||
| ) | ||
|
|
||
| if edges: |
There was a problem hiding this comment.
can we make these variable names more clear - edges = dashboards within the app?
also dashboard_urn being the app's urn is super confusing - let's make this more clear
metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi.py
Outdated
Show resolved
Hide resolved
| app_id: str, | ||
| ) -> Optional[Dict]: | ||
| # [Date: 2024/10/18] As per API doc, the service principal approach is not supported for regular API | ||
| # https://learn.microsoft.com/en-us/rest/api/power-bi/apps/get-app |
There was a problem hiding this comment.
let's add this to the doc on limitations
There was a problem hiding this comment.
It is not going to impact user experience, as we are getting app from AdminApiResolver and I have updated the assets list in doc MD file.
metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/powerbi_api.py
Outdated
Show resolved
Hide resolved
metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/powerbi_api.py
Outdated
Show resolved
Hide resolved
| return | ||
|
|
||
| # Map to find out which dashboards belongs to the App | ||
| dashboard_map: Dict[str, Dict] = { |
There was a problem hiding this comment.
improve variable naming here
| @@ -221,6 +229,7 @@ def get_dashboards(self, workspace: Workspace) -> List[Dashboard]: | |||
| tiles=[], | |||
| users=[], | |||
| tags=[], | |||
| app_reference=None, # It is getting set later from scan_result | |||
There was a problem hiding this comment.
what do we use app_reference for?
There was a problem hiding this comment.
Removed it, not in used
|
respond |
No description provided.