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): powerbi # support platform instance #7583
feat(ingestion): powerbi # support platform instance #7583
Conversation
@@ -878,8 +890,12 @@ def report_to_datahub_work_units( | |||
@platform_name("PowerBI") | |||
@config_class(PowerBiDashboardSourceConfig) | |||
@support_status(SupportStatus.CERTIFIED) | |||
@capability(SourceCapability.DESCRIPTIONS, "Enabled by default") | |||
@capability(SourceCapability.PLATFORM_INSTANCE, "Enabled by default") | |||
@capability(SourceCapability.PLATFORM_INSTANCE, "Enabled by default") |
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.
is this duplicated?
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.
What about platform instances in the urns to upstream datasets parsed from M-Query?
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.
This works... but we probably also need a connection_to_platform_instance map similar to what we've done for looker
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.
Why does the docs site already indicate there is a config called "platform_instance"?
…dique/datahub into master+powerbi-platform-instance
Yup as discussed over slack, I will investigate on PowerBI and will open a separate PR for this |
@mohdsiddique looks like tests are failing |
…ta/datahub-fork into master+powerbi-platform-instance
It was because of master merge, updated golden file for big-query tables |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -878,8 +890,11 @@ def report_to_datahub_work_units( | |||
@platform_name("PowerBI") | |||
@config_class(PowerBiDashboardSourceConfig) | |||
@support_status(SupportStatus.CERTIFIED) | |||
@capability(SourceCapability.DESCRIPTIONS, "Enabled by default") |
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.
Thank you!
|
||
output_path: str = f"{tmp_path}/powerbi_platform_instance_mces.json" | ||
|
||
pipeline = Pipeline.create( |
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.
Thank you!
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.
LGTM - Thank you!
…#7583) Co-authored-by: MohdSiddiqueBagwan <mohdsiddique.bagwan@gslab.com> Co-authored-by: John Joyce <john@acryl.io>
…#7583) Co-authored-by: MohdSiddiqueBagwan <mohdsiddique.bagwan@gslab.com> Co-authored-by: John Joyce <john@acryl.io>
No description provided.