Fixes #22916: Add chart-level lineage for Metabase connector#26778
Fixes #22916: Add chart-level lineage for Metabase connector#26778Ryadlotfimahtal wants to merge 2 commits intoopen-metadata:mainfrom
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review 👍 Approved with suggestions 0 resolved / 1 findingsAdds chart-level lineage support for the Metabase connector. Consider moving the chart entity lookup outside the source_tables loop to avoid repeated lookups and improve performance. 💡 Performance: Chart entity lookup repeated inside source_tables loop📄 ingestion/src/metadata/ingestion/source/dashboard/metabase/metadata.py:459-468 In The pre-existing Suggested fix🤖 Prompt for agentsOptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
| chart_fqn = fqn.build( | ||
| self.metadata, | ||
| entity_type=LineageChart, | ||
| service_name=self.config.serviceName, | ||
| chart_name=str(chart_details.id), | ||
| ) | ||
| chart_entity = self.metadata.get_by_name( | ||
| entity=LineageChart, | ||
| fqn=chart_fqn, | ||
| ) |
There was a problem hiding this comment.
💡 Performance: Chart entity lookup repeated inside source_tables loop
In _yield_lineage_from_query, the chart_fqn/chart_entity lookup (lines 459-468) is inside the for table in lineage_parser.source_tables: loop, but neither depends on table. This means get_by_name is called for the same chart entity on every loop iteration, making a redundant API call per source table.
The pre-existing to_entity (dashboard) lookup has the same issue, so this follows the existing pattern — but the new code doubles the number of redundant API calls per iteration.
Suggested fix:
Move both `to_entity` and `chart_entity` lookups above the `for table in lineage_parser.source_tables:` loop (before line 415), since they only depend on `dashboard_name` and `chart_details.id`, not on `table`.
Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion
Describe your changes:
Fixes #22916
The Metabase connector was only creating lineage between source tables and dashboards, but not between
source tables and individual charts.
In both
_yield_lineage_from_queryand_yield_lineage_from_api, added a lineage edge from the sourcetable to the chart entity in addition to the existing table → dashboard edge.
Validated end-to-end on a local OpenMetadata 1.12.3 instance with a live Metabase connection.
Type of change:
Bug fix
Checklist:
Fixes <issue-number>: <short explanation>