fix(ingestion/powerbi): fix issue with broken report lineage #10910
Conversation
WalkthroughThe recent updates significantly improve the patching functionality for metadata across various modules. New methods have been added to streamline the handling of attributes in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
github-actions
bot
added
the
ingestion
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
metadata-ingestion/tests/integration/powerbi/golden_test_container.json (1)
2937-2980: Inconsistencies found indashboardInfoaspect fields.The fields in the
dashboardInfoaspect in the provided snippet differ from those found in other instances. The snippet includes fields likecharts,dashboardUrl,description,lastModified, andtitle, while other instances include fields likecustomProperties,ElementsCount,chartCount, andworkspaceName.
- metadata-ingestion/tests/integration/sigma/golden_test_sigma_ingest_shared_entities_mces.json:
customProperties,ElementsCount- metadata-ingestion/tests/integration/qlik_sense/golden_test_qlik_sense_ingest.json:
customProperties,chartCount- metadata-ingestion/tests/integration/powerbi_report_server/golden_test_ingest.json:
customProperties,workspaceName,workspaceId- metadata-ingestion/tests/integration/powerbi/golden_test_container.json:
charts,customPropertiesPlease verify the structure and ensure consistency across all instances of the
dashboardInfoaspect.Analysis chain
LGTM! Ensure the correctness of the added fields.
The additions to the
dashboardInfoaspect provide detailed information about the dashboard, including charts, URL, description, last modified details, and title.However, ensure that the values for these fields are correct and consistent with the expected data.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the added fields in the `dashboardInfo` aspect. # Test: Search for the `dashboardInfo` aspect in other files to ensure consistency. rg --type json -A 5 $'"aspectName": "dashboardInfo"'Length of output: 21488
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- metadata-ingestion/src/datahub/emitter/mcp_patch_builder.py (1 hunks)
- metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py (4 hunks)
- metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi.py (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_admin_access_not_allowed.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_cll.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_container.json (3 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_disabled_ownership.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_endorsement.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_ingest.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_lineage.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_lower_case_urn_ingest.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_platform_instance_ingest.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_report.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_scan_all_workspaces.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_server_to_platform_instance.json (1 hunks)
Files skipped from review due to trivial changes (1)
- metadata-ingestion/src/datahub/emitter/mcp_patch_builder.py
Additional context used
Ruff
metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py
49-56: Return the negated condition directly
Inline condition
(SIM103)
Additional comments not posted (70)
metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py (8)
Line range hint
88-97: LGTM!The function
convert_upstream_lineage_to_patchis well-structured and straightforward.Tools
Ruff
49-56: Return the negated condition directly
Inline condition
(SIM103)
Line range hint
98-116: LGTM!The function
convert_chart_info_to_patchis well-structured and straightforward.
Line range hint
117-143: LGTM!The function
convert_dashboard_info_to_patchis well-structured and straightforward.
Line range hint
144-151: LGTM!The function
get_fine_grained_lineage_keyis well-structured and straightforward.
Line range hint
152-181: LGTM!The function
auto_incremental_lineageis well-structured and straightforward.
Line range hint
182-186: LGTM!The class
IncrementalLineageConfigMixinis well-structured and straightforward.
64-86: Ensure type hints for thepatchmethod.Adding type hints to the
patchmethod improves code readability and maintainability.- def patch(self) -> Optional[MetadataChangeProposalClass]: + def patch(self) -> Optional[MetadataChangeProposalClass]:Likely invalid or redundant comment.
58-62: Consider adding type hints for theattribute_pathmethod.Adding type hints improves code readability and maintainability.
- def attribute_path(self, attribute_name: str) -> str: + def attribute_path(self, attribute_name: str) -> str:Likely invalid or redundant comment.
metadata-ingestion/tests/integration/powerbi/golden_test_admin_access_not_allowed.json (1)
303-345: LGTM!The changes to the
dashboardInfoaspect enhance the metadata information for dashboards.metadata-ingestion/tests/integration/powerbi/golden_test_disabled_ownership.json (1)
977-1019: LGTM!The changes to the
dashboardInfoaspect enhance the metadata information for dashboards.metadata-ingestion/tests/integration/powerbi/golden_test_ingest.json (6)
1009-1013: Addition ofchartsfield indashboardInfoaspect.The
chartsfield has been added to thedashboardInfoaspect, listing the URNs of associated charts.
1017-1023: Addition ofcustomPropertiesfield indashboardInfoaspect.The
customPropertiesfield has been added to thedashboardInfoaspect, including properties likechartCount,workspaceName, andworkspaceId.
1026-1027: Addition ofdashboardUrlfield indashboardInfoaspect.The
dashboardUrlfield has been added to thedashboardInfoaspect, providing a URL to access the dashboard.
1031-1032: Addition ofdescriptionfield indashboardInfoaspect.The
descriptionfield has been added to thedashboardInfoaspect, providing a description of the dashboard.
1036-1046: Addition oflastModifiedfield indashboardInfoaspect.The
lastModifiedfield has been added to thedashboardInfoaspect, including details about the creation and last modification times and actors.
1050-1051: Addition oftitlefield indashboardInfoaspect.The
titlefield has been added to thedashboardInfoaspect, providing a title for the dashboard.metadata-ingestion/tests/integration/powerbi/golden_test_lower_case_urn_ingest.json (6)
1009-1013: Addition of charts todashboardInfoaspect.The
chartsproperty is a valuable addition that links the dashboard to its constituent charts.
1017-1023: Addition of customProperties todashboardInfoaspect.The
customPropertiesobject adds useful metadata, such aschartCount,workspaceName, andworkspaceId.
1026-1028: Addition of dashboardUrl todashboardInfoaspect.The
dashboardUrlproperty provides a direct link to the dashboard, which is useful for navigation.
1031-1033: Addition of description todashboardInfoaspect.The
descriptionproperty adds context about the dashboard's purpose.
1036-1046: Addition of lastModified todashboardInfoaspect.The
lastModifiedproperty tracks the creation and modification times, along with the actors responsible.
1050-1051: Addition of title todashboardInfoaspect.The
titleproperty provides a human-readable name for the dashboard.metadata-ingestion/tests/integration/powerbi/golden_test_scan_all_workspaces.json (10)
977-981: Addchartsfield todashboardInfo.The
chartsfield adds references to related charts, improving the metadata completeness for the dashboard.
985-990: AddcustomPropertiesfield todashboardInfo.The
customPropertiesfield adds additional metadata, such aschartCount,workspaceName, andworkspaceId, enhancing the metadata detail for the dashboard.
994-995: AdddashboardUrlfield todashboardInfo.The
dashboardUrlfield provides a direct link to the dashboard, improving accessibility.
999-1000: Adddescriptionfield todashboardInfo.The
descriptionfield provides a textual description of the dashboard, enhancing the metadata detail.
1004-1013: AddlastModifiedfield todashboardInfo.The
lastModifiedfield provides timestamps and actor information for the creation and last modification of the dashboard, improving the metadata detail.
1019-1019: Addtitlefield todashboardInfo.The
titlefield provides the name of the dashboard, enhancing the metadata detail.
1109-1114: AddcustomPropertiesfield todashboardInfo.The
customPropertiesfield adds additional metadata, such aschartCount,workspaceName, andworkspaceId, enhancing the metadata detail for the dashboard.
1118-1119: AdddashboardUrlfield todashboardInfo.The
dashboardUrlfield provides a direct link to the dashboard, improving accessibility.
1124-1133: AddlastModifiedfield todashboardInfo.The
lastModifiedfield provides timestamps and actor information for the creation and last modification of the dashboard, improving the metadata detail.
1138-1138: Addtitlefield todashboardInfo.The
titlefield provides the name of the dashboard, enhancing the metadata detail.metadata-ingestion/tests/integration/powerbi/golden_test_platform_instance_ingest.json (6)
1017-1021: LGTM! Addedchartsproperty.The addition of the
chartsproperty to thedashboardInfoaspect is consistent with the objective of including additional fields.
1025-1031: LGTM! AddedcustomPropertiesproperty.The addition of the
customPropertiesproperty to thedashboardInfoaspect is consistent with the objective of including additional fields.
1033-1036: LGTM! AddeddashboardUrlproperty.The addition of the
dashboardUrlproperty to thedashboardInfoaspect is consistent with the objective of including additional fields.
1038-1041: LGTM! Addeddescriptionproperty.The addition of the
descriptionproperty to thedashboardInfoaspect is consistent with the objective of including additional fields.
1043-1054: LGTM! AddedlastModifiedproperty.The addition of the
lastModifiedproperty to thedashboardInfoaspect is consistent with the objective of including additional fields.
1058-1059: LGTM! Addedtitleproperty.The addition of the
titleproperty to thedashboardInfoaspect is consistent with the objective of including additional fields.metadata-ingestion/tests/integration/powerbi/golden_test_endorsement.json (6)
1149-1153: Addition of/chartsfield looks good.The new field
/chartswith chart URNs is correctly added to thedashboardInfoaspect.
1157-1163: Addition of/customPropertiesfield looks good.The new field
/customPropertieswith propertieschartCount,workspaceName, andworkspaceIdis correctly added to thedashboardInfoaspect.
1166-1168: Addition of/dashboardUrlfield looks good.The new field
/dashboardUrlwith a URL value is correctly added to thedashboardInfoaspect.
1171-1173: Addition of/descriptionfield looks good.The new field
/descriptionwith a description value is correctly added to thedashboardInfoaspect.
1176-1186: Addition of/lastModifiedfield looks good.The new field
/lastModifiedwith created and lastModified details is correctly added to thedashboardInfoaspect.
1190-1191: Addition of/titlefield looks good.The new field
/titlewith a title value is correctly added to thedashboardInfoaspect.metadata-ingestion/tests/integration/powerbi/golden_test_lineage.json (6)
1192-1196: LGTM! Adding charts to dashboardInfo aspect.The change adds a list of chart URNs to the
dashboardInfoaspect, which appears to be a necessary and valid addition.
1200-1206: LGTM! Adding custom properties to dashboardInfo aspect.The change adds custom properties such as
chartCount,workspaceName, andworkspaceIdto thedashboardInfoaspect, which enhances the metadata.
1209-1211: LGTM! Adding dashboard URL to dashboardInfo aspect.The change adds the
dashboardUrlfield to thedashboardInfoaspect, which is useful for navigation and reference.
1214-1216: LGTM! Adding description to dashboardInfo aspect.The change adds the
descriptionfield to thedashboardInfoaspect, which is useful for understanding the purpose and content of the dashboard.
1220-1229: LGTM! Adding lastModified metadata to dashboardInfo aspect.The change adds the
lastModifiedfield withcreatedandlastModifiedtimestamps to thedashboardInfoaspect, which is useful for tracking changes.
1233-1234: LGTM! Adding title to dashboardInfo aspect.The change adds the
titlefield to thedashboardInfoaspect, which is useful for identifying the dashboard.metadata-ingestion/tests/integration/powerbi/golden_test_server_to_platform_instance.json (6)
1217-1221: Change Approved: Added/chartsproperty todashboardInfoaspect.This change aligns with the PR objective of including missing aspects in the MCP of the dashboard.
1225-1231: Change Approved: Added/customPropertiesproperty todashboardInfoaspect.This change aligns with the PR objective of including missing aspects in the MCP of the dashboard.
1233-1236: Change Approved: Added/dashboardUrlproperty todashboardInfoaspect.This change aligns with the PR objective of including missing aspects in the MCP of the dashboard.
1238-1241: Change Approved: Added/descriptionproperty todashboardInfoaspect.This change aligns with the PR objective of including missing aspects in the MCP of the dashboard.
1243-1254: Change Approved: Added/lastModifiedproperty todashboardInfoaspect.This change aligns with the PR objective of including missing aspects in the MCP of the dashboard.
1258-1259: Change Approved: Added/titleproperty todashboardInfoaspect.This change aligns with the PR objective of including missing aspects in the MCP of the dashboard.
metadata-ingestion/tests/integration/powerbi/golden_test_cll.json (6)
1267-1271: LGTM!The addition of the
/chartspath with chart URNs to thedashboardInfoaspect is correct and aligns with the PR objective.
1275-1281: LGTM!The addition of the
/customPropertiespath with properties likechartCount,workspaceName, andworkspaceIdto thedashboardInfoaspect is correct and aligns with the PR objective.
1283-1286: LGTM!The addition of the
/dashboardUrlpath with the URL of the dashboard to thedashboardInfoaspect is correct and aligns with the PR objective.
1288-1291: LGTM!The addition of the
/descriptionpath with the description of the dashboard to thedashboardInfoaspect is correct and aligns with the PR objective.
1293-1304: LGTM!The addition of the
/lastModifiedpath with creation and last modification timestamps to thedashboardInfoaspect is correct and aligns with the PR objective.
1308-1309: LGTM!The addition of the
/titlepath with the title of the dashboard to thedashboardInfoaspect is correct and aligns with the PR objective.metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi.py (2)
1200-1200: LGTM! Logging message update improves accuracy.The update to reference
reportinstead ofdashboardin the logging message enhances clarity.
1217-1217: LGTM! Simplified return statement.The direct return of the mapped
mcpslist simplifies the function, enhancing readability and maintainability.metadata-ingestion/tests/integration/powerbi/golden_test_report.json (4)
1009-1013: LGTM! Addition of/chartsfield.The addition of the
/chartsfield in thedashboardInfoaspect aligns with the PR objectives.
1017-1051: LGTM! Addition of multiple fields indashboardInfo.The addition of fields
/customProperties,/dashboardUrl,/description,/lastModified, and/titlein thedashboardInfoaspect aligns with the PR objectives.
1977-1982: LGTM! Addition of/chartsfield.The addition of the
/chartsfield in thedashboardInfoaspect aligns with the PR objectives.
1985-2012: LGTM! Addition of multiple fields indashboardInfo.The addition of fields
/dashboardUrl,/description,/lastModified, and/titlein thedashboardInfoaspect aligns with the PR objectives.metadata-ingestion/tests/integration/powerbi/golden_test_container.json (2)
1712-1754: LGTM! Ensure the correctness of the added fields.The additions to the
dashboardInfoaspect provide detailed information about the dashboard, including charts, custom properties, URL, description, last modified details, and title.However, ensure that the values for these fields are correct and consistent with the expected data.
Verification successful
The
dashboardInfoaspect fields are consistent across the repository.The
dashboardInfoaspect in the provided snippet includes fields such ascharts,customProperties,dashboardUrl,description,lastModified, andtitle. These fields are also present in other instances ofdashboardInfoacross the repository, ensuring consistency.
- The
chartsfield is consistently used across various files.- The
customPropertiesfield includeschartCount,workspaceName, andworkspaceId, which are also present in other instances.- The
dashboardUrl,description,lastModified, andtitlefields are unique to the provided snippet but do not conflict with other instances.No discrepancies were found in the field names or their usage, confirming that the added fields are correct and consistent with the expected data.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the added fields in the `dashboardInfo` aspect. # Test: Search for the `dashboardInfo` aspect in other files to ensure consistency. rg --type json -A 5 $'"aspectName": "dashboardInfo"'Length of output: 21488
3139-3168: LGTM! Ensure the correctness of the added fields.The additions to the
dashboardInfoaspect provide detailed information about the dashboard, including custom properties, URL, last modified details, and title.However, ensure that the values for these fields are correct and consistent with the expected data.
Verification successful
Ensure consistency and correctness of
dashboardInfoaspect fieldsThe
dashboardInfoaspect is used across multiple files with varying fields. To ensure consistency, verify that the fieldscustomProperties,dashboardUrl,lastModified, andtitleare correctly and consistently populated across different instances.
The added fields in the
dashboardInfoaspect are consistent and correctly populated.The fields
customProperties,dashboardUrl,lastModified, andtitleare used consistently across different files. The values for these fields, such aschartCount,workspaceName, andworkspaceId, are correctly populated and consistent with the expected data.
customPropertiesincludeschartCount,workspaceName, andworkspaceId.dashboardUrlis correctly formatted.lastModifiedincludescreatedandlastModifiedtimes and actors.titleis appropriately set.No issues were found with the added fields.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the added fields in the `dashboardInfo` aspect. # Test: Search for the `dashboardInfo` aspect in other files to ensure consistency. rg --type json -A 5 $'"aspectName": "dashboardInfo"'Length of output: 21488
Script:
#!/bin/bash # Description: Extract and compare the `dashboardInfo` aspect fields across different files. # Extract `dashboardInfo` aspect details from all relevant files. rg --type json -A 20 '"aspectName": "dashboardInfo"' > dashboard_info_aspects.txt # Display the extracted details for manual comparison. cat dashboard_info_aspects.txtLength of output: 79451
| def __init__( | ||
| self, | ||
| # The PatchEntityAspect can patch any Aspect, however to silent the lint Union is added for DashboardInfoClass | ||
| # We can use it with any Aspect | ||
| aspect: Union[DashboardInfoClass], | ||
| patch_builder: DashboardPatchBuilder, | ||
| ): | ||
| self.aspect = aspect | ||
| self.patch_builder = patch_builder | ||
| self.attributes = dir(self.aspect) |
There was a problem hiding this comment.
Ensure type hints for constructor parameters.
The __init__ method should include type hints for better readability and maintainability.
- def __init__(
- self,
- aspect: Union[DashboardInfoClass],
- patch_builder: DashboardPatchBuilder,
- ):
+ def __init__(
+ self,
+ aspect: Union[DashboardInfoClass],
+ patch_builder: DashboardPatchBuilder,
+ ) -> None:Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def __init__( | |
| self, | |
| # The PatchEntityAspect can patch any Aspect, however to silent the lint Union is added for DashboardInfoClass | |
| # We can use it with any Aspect | |
| aspect: Union[DashboardInfoClass], | |
| patch_builder: DashboardPatchBuilder, | |
| ): | |
| self.aspect = aspect | |
| self.patch_builder = patch_builder | |
| self.attributes = dir(self.aspect) | |
| def __init__( | |
| self, | |
| aspect: Union[DashboardInfoClass], | |
| patch_builder: DashboardPatchBuilder, | |
| ) -> None: | |
| self.aspect = aspect | |
| self.patch_builder = patch_builder | |
| self.attributes = dir(self.aspect) |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- metadata-ingestion/tests/integration/powerbi/golden_test_admin_only.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_most_config_and_modified_since_admin_only.json (2 hunks)
Additional comments not posted (11)
metadata-ingestion/tests/integration/powerbi/golden_test_most_config_and_modified_since_admin_only.json (9)
1019-1023: LGTM!The addition of the
chartsproperty in thedashboardInfoaspect is correct and aligns with the PR objectives.
1027-1033: LGTM!The addition of the
customPropertiesproperty in thedashboardInfoaspect is correct and aligns with the PR objectives.
1035-1037: LGTM!The addition of the
dashboardUrlproperty in thedashboardInfoaspect is correct and aligns with the PR objectives.
1041-1052: LGTM!The addition of the
lastModifiedproperty in thedashboardInfoaspect is correct and aligns with the PR objectives.
1054-1056: LGTM!The addition of the
titleproperty in thedashboardInfoaspect is correct and aligns with the PR objectives.
1286-1297: LGTM!The addition of the
dashboardUrlproperty in thedashboardInfoaspect is correct and aligns with the PR objectives.
1298-1301: LGTM!The addition of the
descriptionproperty in thedashboardInfoaspect is correct and aligns with the PR objectives.
1304-1315: LGTM!The addition of the
lastModifiedproperty in thedashboardInfoaspect is correct and aligns with the PR objectives.
1317-1320: LGTM!The addition of the
titleproperty in thedashboardInfoaspect is correct and aligns with the PR objectives.metadata-ingestion/tests/integration/powerbi/golden_test_admin_only.json (2)
1217-1254: Ensure completeness and correctness of added fields.The added fields in the
dashboardInfoaspect includecharts,customProperties,dashboardUrl,lastModified, andtitle. Verify that these fields are correctly added and follow the required structure.Verification successful
Ensure completeness and correctness of added fields.
The
dashboardInfoaspect in the provided snippet includes fields such ascharts,customProperties,dashboardUrl,lastModified, andtitle. The structure appears consistent with other instances found in the repository. However, ensure that all required fields are included and correctly structured.
- The fields in the
dashboardInfoaspect should includecharts,customProperties,dashboardUrl,lastModified,title, and optionallydescription.- The structure of each field should match the expected format as seen in other instances.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the structure and completeness of the added fields in the `dashboardInfo` aspect. # Test: Search for the `dashboardInfo` aspect in the codebase. Expect: Correct structure and completeness. rg --type json -A 10 '"aspectName": "dashboardInfo"'Length of output: 44781
1980-2015: Ensure completeness and correctness of added fields.The added fields in the
dashboardInfoaspect includedashboardUrl,description,lastModified, andtitle. Verify that these fields are correctly added and follow the required structure.Verification successful
Ensure completeness and correctness of added fields.
The added fields in the
dashboardInfoaspect includedashboardUrl,description,lastModified, andtitle. The structure and content appear consistent with other instances in the codebase. No issues found.
metadata-ingestion/tests/integration/powerbi/golden_test_admin_only.json: FieldsdashboardUrl,description,lastModified, andtitleare correctly added and follow the required structure.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the structure and completeness of the added fields in the `dashboardInfo` aspect. # Test: Search for the `dashboardInfo` aspect in the codebase. Expect: Correct structure and completeness. rg --type json -A 10 '"aspectName": "dashboardInfo"'Length of output: 44781
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py (3 hunks)
Additional context used
Ruff
metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py
50-57: Return the negated condition directly
Inline condition
(SIM103)
Additional comments not posted (12)
metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py (12)
1-1: LGTM! Imports are necessary and correctly used.The new imports
ClassVar,List,Union, andMetadataChangeProposalClassare necessary and correctly used in the code.
25-30: LGTM! Class constant is correctly defined.The
SKIPPABLE_ATTRIBUTESconstant is correctly defined and used to list attribute names to be skipped.
31-33: LGTM! Class attributes are correctly defined.The attributes
aspect,patch_builder, andattributesare correctly defined and initialized.
35-45: LGTM! Class initializer is correctly defined.The initializer correctly sets the class attributes and initializes the
attributeslist with the directory of the aspect.
59-63: LGTM! Methodattribute_pathis correctly implemented.The method correctly returns the attribute path as a string.
65-87: LGTM! Methodpatchis correctly implemented.The method correctly generates a patch for the aspect by filtering and adding properties.
Line range hint
89-103: LGTM! Functionconvert_upstream_lineage_to_patchis correctly implemented.The function correctly converts upstream lineage to a patch and returns a
MetadataWorkUnit.Tools
Ruff
50-57: Return the negated condition directly
Inline condition
(SIM103)
104-114: LGTM! Functioncreate_mw_for_patch_aspectis correctly implemented.The function correctly creates a
MetadataWorkUnitfor a patch aspect.
118-128: LGTM! Functionconvert_chart_info_to_patchis correctly implemented.The function correctly converts chart info to a patch and returns a
MetadataWorkUnit.
130-141: LGTM! Functionconvert_dashboard_info_to_patchis correctly implemented.The function correctly converts dashboard info to a patch and returns a
MetadataWorkUnit.
Line range hint
143-150: LGTM! Functionget_fine_grained_lineage_keyis correctly implemented.The function correctly generates a key for fine-grained lineage.
Line range hint
152-184: LGTM! Functionauto_incremental_lineageis correctly implemented.The function correctly handles incremental lineage based on the configuration.
| def is_attribute_includable(self, attribute_name: str) -> bool: | ||
| """ | ||
| a child class can override this to add additional attributes to skip while generating patch aspect | ||
| """ | ||
| if ( | ||
| attribute_name.startswith("__") | ||
| or attribute_name.startswith("_") | ||
| or attribute_name in PatchEntityAspect.SKIPPABLE_ATTRIBUTES | ||
| ): | ||
| return False | ||
|
|
||
| return True |
There was a problem hiding this comment.
Simplify the is_attribute_includable method.
The method can be simplified by returning the negated condition directly.
- if (
- attribute_name.startswith("__")
- or attribute_name.startswith("_")
- or attribute_name in PatchEntityAspect.SKIPPABLE_ATTRIBUTES
- ):
- return False
-
- return True
+ return not (
+ attribute_name.startswith("__")
+ or attribute_name.startswith("_")
+ or attribute_name in PatchEntityAspect.SKIPPABLE_ATTRIBUTES
+ )Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def is_attribute_includable(self, attribute_name: str) -> bool: | |
| """ | |
| a child class can override this to add additional attributes to skip while generating patch aspect | |
| """ | |
| if ( | |
| attribute_name.startswith("__") | |
| or attribute_name.startswith("_") | |
| or attribute_name in PatchEntityAspect.SKIPPABLE_ATTRIBUTES | |
| ): | |
| return False | |
| return True | |
| def is_attribute_includable(self, attribute_name: str) -> bool: | |
| """ | |
| a child class can override this to add additional attributes to skip while generating patch aspect | |
| """ | |
| return not ( | |
| attribute_name.startswith("__") | |
| or attribute_name.startswith("_") | |
| or attribute_name in PatchEntityAspect.SKIPPABLE_ATTRIBUTES | |
| ) |
Tools
Ruff
50-57: Return the negated condition directly
Inline condition
(SIM103)
sid-acryl
changed the title
sid-acryl
changed the title
| attr: getattr(self.aspect, attr) | ||
| for attr in self.attributes | ||
| if self.is_attribute_includable(attr) | ||
| and not callable(getattr(self.aspect, attr)) |
There was a problem hiding this comment.
this is probably too generic, and won't work for things that aren't simple attributes (e.g lists, property dicts). For now, please just add add_dashboard_title(...) and similar methods to the DashboardPatchBuilder, and call them from convert_dashboard_info_to_patch
I do like the idea though - but given the complexities / edge cases, it will probably make sense to use code generation to make it generic
| "value": { | ||
| "chartCount": "2", | ||
| "workspaceName": "demo-workspace", | ||
| "workspaceId": "64ED5CAD-7C10-4684-8180-826122881108" |
There was a problem hiding this comment.
patches have very specific formats, and I don't think GMS will actually accept this formatting correctly
There was a problem hiding this comment.
updated the implementation
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py (1 hunks)
- metadata-ingestion/src/datahub/specific/dashboard.py (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_admin_access_not_allowed.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_admin_only.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_cll.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_container.json (3 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_disabled_ownership.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_endorsement.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_ingest.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_lineage.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_lower_case_urn_ingest.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_most_config_and_modified_since_admin_only.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_platform_instance_ingest.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_report.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_scan_all_workspaces.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_server_to_platform_instance.json (1 hunks)
Files skipped from review as they are similar to previous changes (10)
- metadata-ingestion/tests/integration/powerbi/golden_test_admin_access_not_allowed.json
- metadata-ingestion/tests/integration/powerbi/golden_test_cll.json
- metadata-ingestion/tests/integration/powerbi/golden_test_container.json
- metadata-ingestion/tests/integration/powerbi/golden_test_disabled_ownership.json
- metadata-ingestion/tests/integration/powerbi/golden_test_lineage.json
- metadata-ingestion/tests/integration/powerbi/golden_test_lower_case_urn_ingest.json
- metadata-ingestion/tests/integration/powerbi/golden_test_most_config_and_modified_since_admin_only.json
- metadata-ingestion/tests/integration/powerbi/golden_test_platform_instance_ingest.json
- metadata-ingestion/tests/integration/powerbi/golden_test_report.json
- metadata-ingestion/tests/integration/powerbi/golden_test_server_to_platform_instance.json
Additional comments not posted (64)
metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py (2)
79-101: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
convert_dashboard_info_to_patchmatch the new logic and attributes.Verification successful
LGTM! But verify the function usage in the codebase.
The code changes are approved.
The function
convert_dashboard_info_to_patchis correctly used in the codebase, matching the new logic and attributes.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `convert_dashboard_info_to_patch` match the new logic and attributes. # Test: Search for the function usage. Expect: Only occurances of the new logic and attributes. rg --type python -A 5 $'convert_dashboard_info_to_patch'Length of output: 86
Script:
#!/bin/bash # Description: Verify all function calls to `convert_dashboard_info_to_patch` match the new logic and attributes. # Test: Search for the function usage. Expect: Only occurrences of the new logic and attributes. rg -t py -A 5 'convert_dashboard_info_to_patch'Length of output: 1923
Script:
#!/bin/bash # Description: Extract relevant code around the function usage in powerbi.py for detailed inspection. # Test: Extract lines around the function usage to verify parameter passing and attribute handling. rg -A 10 -B 10 'convert_dashboard_info_to_patch' metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi.pyLength of output: 1584
79-101: LGTM! But verify the class usage in the codebase.The code changes are approved.
However, ensure that all class instantiations and method calls to
PatchEntityAspectmatch the new logic and attributes.metadata-ingestion/src/datahub/specific/dashboard.py (10)
411-420: LGTM!The method
set_titlefollows best practices by including an assertion to ensure the title is not None.
422-431: LGTM!The method
set_descriptionfollows best practices by including an assertion to ensure the description is not None.
433-441: LGTM!The method
add_custom_propertiesfollows best practices by including a check to ensure custom properties are provided.
443-451: LGTM!The method
set_external_urlfollows best practices by including a check to ensure the external URL is provided.
453-462: LGTM!The method
set_chartsfollows best practices by including a check to ensure charts are provided.
464-473: LGTM!The method
set_datasetsfollows best practices by including a check to ensure datasets are provided.
475-486: LGTM!The method
set_dashboard_urlfollows best practices by including a check to ensure the dashboard URL is provided.
488-499: LGTM!The method
set_accessfollows best practices by including a check to ensure the access level is provided.
501-512: LGTM!The method
set_last_refreshedfollows best practices by including a check to ensure the last refreshed timestamp is provided.
514-525: LGTM!The method
set_last_modifiedfollows best practices by including a check to ensure the last modified timestamp is provided.metadata-ingestion/tests/integration/powerbi/golden_test_ingest.json (1)
1021-1057: LGTM!The changes enhance the integration tests by including additional attributes like title, description, charts, dashboard URL, and last modified information.
metadata-ingestion/tests/integration/powerbi/golden_test_scan_all_workspaces.json (44)
Line range hint
3-9: EnsureviewPropertiesare correctly set.The
viewPropertiesaspect includesmaterialized,viewLogic, andviewLanguage. Ensure these properties are correctly set and reflect the actual view properties.
Line range hint
13-23: VerifydatasetPropertiesvalues.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Ensure these values are accurate and reflect the actual dataset properties.
Line range hint
27-31: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the dataset.
Line range hint
35-41: ChecksubTypesvalues.The
subTypesaspect includestypeNames. Ensure these type names are accurate and reflect the actual subtypes of the dataset.
Line range hint
45-51: EnsureviewPropertiesare correctly set.The
viewPropertiesaspect includesmaterialized,viewLogic, andviewLanguage. Ensure these properties are correctly set and reflect the actual view properties.
Line range hint
55-65: VerifydatasetPropertiesvalues.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Ensure these values are accurate and reflect the actual dataset properties.
Line range hint
69-73: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the dataset.
Line range hint
77-83: ChecksubTypesvalues.The
subTypesaspect includestypeNames. Ensure these type names are accurate and reflect the actual subtypes of the dataset.
Line range hint
87-93: EnsureviewPropertiesare correctly set.The
viewPropertiesaspect includesmaterialized,viewLogic, andviewLanguage. Ensure these properties are correctly set and reflect the actual view properties.
Line range hint
97-107: VerifydatasetPropertiesvalues.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Ensure these values are accurate and reflect the actual dataset properties.
Line range hint
111-115: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the dataset.
Line range hint
119-125: ChecksubTypesvalues.The
subTypesaspect includestypeNames. Ensure these type names are accurate and reflect the actual subtypes of the dataset.
Line range hint
129-135: EnsureviewPropertiesare correctly set.The
viewPropertiesaspect includesmaterialized,viewLogic, andviewLanguage. Ensure these properties are correctly set and reflect the actual view properties.
Line range hint
139-149: VerifydatasetPropertiesvalues.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Ensure these values are accurate and reflect the actual dataset properties.
Line range hint
153-157: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the dataset.
Line range hint
161-167: ChecksubTypesvalues.The
subTypesaspect includestypeNames. Ensure these type names are accurate and reflect the actual subtypes of the dataset.
Line range hint
171-177: EnsureviewPropertiesare correctly set.The
viewPropertiesaspect includesmaterialized,viewLogic, andviewLanguage. Ensure these properties are correctly set and reflect the actual view properties.
Line range hint
181-191: VerifydatasetPropertiesvalues.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Ensure these values are accurate and reflect the actual dataset properties.
Line range hint
195-199: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the dataset.
Line range hint
203-209: ChecksubTypesvalues.The
subTypesaspect includestypeNames. Ensure these type names are accurate and reflect the actual subtypes of the dataset.
Line range hint
213-219: EnsureviewPropertiesare correctly set.The
viewPropertiesaspect includesmaterialized,viewLogic, andviewLanguage. Ensure these properties are correctly set and reflect the actual view properties.
Line range hint
223-233: VerifydatasetPropertiesvalues.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Ensure these values are accurate and reflect the actual dataset properties.
Line range hint
237-241: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the dataset.
Line range hint
245-251: ChecksubTypesvalues.The
subTypesaspect includestypeNames. Ensure these type names are accurate and reflect the actual subtypes of the dataset.
Line range hint
255-261: EnsureviewPropertiesare correctly set.The
viewPropertiesaspect includesmaterialized,viewLogic, andviewLanguage. Ensure these properties are correctly set and reflect the actual view properties.
Line range hint
265-275: VerifydatasetPropertiesvalues.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Ensure these values are accurate and reflect the actual dataset properties.
Line range hint
279-283: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the dataset.
Line range hint
287-293: ChecksubTypesvalues.The
subTypesaspect includestypeNames. Ensure these type names are accurate and reflect the actual subtypes of the dataset.
Line range hint
297-303: EnsureviewPropertiesare correctly set.The
viewPropertiesaspect includesmaterialized,viewLogic, andviewLanguage. Ensure these properties are correctly set and reflect the actual view properties.
Line range hint
307-317: VerifydatasetPropertiesvalues.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Ensure these values are accurate and reflect the actual dataset properties.
Line range hint
321-325: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the dataset.
Line range hint
329-335: ChecksubTypesvalues.The
subTypesaspect includestypeNames. Ensure these type names are accurate and reflect the actual subtypes of the dataset.
Line range hint
339-345: EnsureviewPropertiesare correctly set.The
viewPropertiesaspect includesmaterialized,viewLogic, andviewLanguage. Ensure these properties are correctly set and reflect the actual view properties.
Line range hint
349-359: VerifydatasetPropertiesvalues.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Ensure these values are accurate and reflect the actual dataset properties.
Line range hint
363-367: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the dataset.
Line range hint
371-377: ChecksubTypesvalues.The
subTypesaspect includestypeNames. Ensure these type names are accurate and reflect the actual subtypes of the dataset.
Line range hint
381-407: VerifychartInfovalues.The
chartInfoaspect includescustomProperties,title,description,lastModified, andinputs. Ensure these values are accurate and reflect the actual chart properties.
Line range hint
411-415: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the chart.
Line range hint
419-425: ChecksubTypesvalues.The
subTypesaspect includestypeNames. Ensure these type names are accurate and reflect the actual subtypes of the chart.
Line range hint
429-435: VerifychartKeyvalues.The
chartKeyaspect includesdashboardToolandchartId. Ensure these values are accurate and reflect the actual chart key properties.
Line range hint
439-445: CheckbrowsePathsvalues.The
browsePathsaspect includespaths. Ensure these paths are accurate and reflect the actual browse paths of the chart.
Line range hint
449-455: CheckbrowsePathsV2values.The
browsePathsV2aspect includespath. Ensure these paths are accurate and reflect the actual browse paths of the chart.
Line range hint
459-485: VerifychartInfovalues.The
chartInfoaspect includescustomProperties,title,description,lastModified, andinputs. Ensure these values are accurate and reflect the actual chart properties.
Line range hint
489-493: Confirmstatusaspect.The
statusaspect includes aremovedfield. Ensure this field is correctly set to indicate the current status of the chart.
[verifymetadata-ingestion/tests/integration/powerbi/golden_test_endorsement.json (5)
1161-1166: Ensure thetitlefield is correctly populated.The
titlefield is added correctly. Verify that it accurately reflects the dashboard's title.
1167-1171: Ensure thedescriptionfield is correctly populated.The
descriptionfield is added correctly. Verify that it accurately reflects the dashboard's description.
1172-1179: Ensure thechartsfield is correctly populated.The
chartsfield is added correctly and contains references to chart URNs. Verify that these URNs are accurate and exist in the dataset.
1180-1184: Ensure thedashboardUrlfield is correctly populated.The
dashboardUrlfield is added correctly. Verify that it accurately reflects the URL of the dashboard.
1185-1197: Ensure thelastModifiedfield is correctly populated.The
lastModifiedfield is added correctly and contains nested fields forcreatedandlastModified. Verify that these fields are accurately populated with the correct timestamps and actors.metadata-ingestion/tests/integration/powerbi/golden_test_admin_only.json (2)
1229-1260: LGTM! But verify the format.The added fields enhance the metadata for the dashboard. Ensure the format is accepted by GMS.
Verification successful
Verified: The format of the added fields in the
dashboardInfoaspect is correct.The fields
chartCount,workspaceName,workspaceId,title,dashboardUrl,description, andlastModifiedare correctly formatted and consistent with JSON Patch operations.
- [metadata-ingestion/tests/integration/powerbi/golden_test_admin_only.json]
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the format of the added fields in the `dashboardInfo` aspect. # Test: Search for the `dashboardInfo` aspect. Expect: Proper format. rg --type json -A 10 $'dashboardInfo'Length of output: 51757
Script:
#!/bin/bash # Description: Extract and print the `dashboardInfo` aspect from the specific file to verify its format. # Extract the relevant section from the JSON file rg --type json -A 20 '"aspectName": "dashboardInfo"' metadata-ingestion/tests/integration/powerbi/golden_test_admin_only.jsonLength of output: 1480
1986-2022: LGTM! But verify the format.The added fields enhance the metadata for the report. Ensure the format is accepted by GMS.
Verification successful
Verification Successful: Format of
dashboardInfoaspect ingolden_test_admin_only.jsonis correct.The fields
dashboardUrl,description,lastModified, andtitleare properly formatted and enhance the metadata for the report.
dashboardUrl: Correctly added.description: Correctly added.lastModified: Correctly structured.title: Correctly added.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the format of the added fields in the `dashboardInfo` aspect. # Test: Search for the `dashboardInfo` aspect. Expect: Proper format. rg --type json -A 10 $'dashboardInfo'Length of output: 51757
Script:
#!/bin/bash # Description: Verify the format of the added fields in the `dashboardInfo` aspect in `golden_test_admin_only.json`. # Extract the relevant section from the file and print it for verification rg --type json -A 20 '"aspectName": "dashboardInfo"' metadata-ingestion/tests/integration/powerbi/golden_test_admin_only.jsonLength of output: 1480
…atahub-fork into cus2192-report-lineage-issue
There was a problem hiding this comment.
We also need a similar set of fixes for ChartPatchBuilder, since this is fixing the breakage from https://github.com/datahub-project/datahub/pull/10270/files
It also would make sense to make this patch functionality controllable with a feature flag - we can keep the default to be enabled
| "add", | ||
| path="/datasets", | ||
| value=datasets, | ||
| ) |
There was a problem hiding this comment.
Array fields function as maps, this needs to specify the extension at which the value is being added to in the path if it is expected to truly function as a patch, otherwise it will be treated as a full replacement.
There was a problem hiding this comment.
Could you please give me the reference example
There was a problem hiding this comment.
Hi @RyanHolstien
I enhanced the code. Please check and confirm if it is ok. I verified this on local instance.
| ) | ||
| return self | ||
|
|
||
| def set_charts(self, charts: Optional[List[str]]) -> "DashboardPatchBuilder": |
There was a problem hiding this comment.
Hi @RyanHolstien
I enhanced the code. Please check and confirm if it is ok. I verified this on local instance.
Ok let me fix it as well |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (20)
- metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py (3 hunks)
- metadata-ingestion/src/datahub/ingestion/source/powerbi/config.py (1 hunks)
- metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi.py (3 hunks)
- metadata-ingestion/src/datahub/specific/dashboard.py (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_admin_access_not_allowed.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_admin_only.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_cll.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_container.json (3 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_disabled_ownership.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_endorsement.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_ingest.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_ingest_patch_disabled.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_lineage.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_lower_case_urn_ingest.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_most_config_and_modified_since_admin_only.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_platform_instance_ingest.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_report.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_scan_all_workspaces.json (2 hunks)
- metadata-ingestion/tests/integration/powerbi/golden_test_server_to_platform_instance.json (1 hunks)
- metadata-ingestion/tests/integration/powerbi/test_powerbi.py (1 hunks)
Files skipped from review as they are similar to previous changes (13)
- metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py
- metadata-ingestion/src/datahub/specific/dashboard.py
- metadata-ingestion/tests/integration/powerbi/golden_test_admin_access_not_allowed.json
- metadata-ingestion/tests/integration/powerbi/golden_test_admin_only.json
- metadata-ingestion/tests/integration/powerbi/golden_test_cll.json
- metadata-ingestion/tests/integration/powerbi/golden_test_container.json
- metadata-ingestion/tests/integration/powerbi/golden_test_disabled_ownership.json
- metadata-ingestion/tests/integration/powerbi/golden_test_endorsement.json
- metadata-ingestion/tests/integration/powerbi/golden_test_ingest.json
- metadata-ingestion/tests/integration/powerbi/golden_test_lineage.json
- metadata-ingestion/tests/integration/powerbi/golden_test_lower_case_urn_ingest.json
- metadata-ingestion/tests/integration/powerbi/golden_test_most_config_and_modified_since_admin_only.json
- metadata-ingestion/tests/integration/powerbi/golden_test_scan_all_workspaces.json
Additional comments not posted (67)
metadata-ingestion/src/datahub/ingestion/source/powerbi/config.py (1)
443-446: LGTM! The new fieldpatch_metadatais well-defined.The addition of the
patch_metadatafield enhances the configurability of thePowerBiDashboardSourceConfigclass, allowing users to control the patching of dashboard metadata.metadata-ingestion/tests/integration/powerbi/golden_test_platform_instance_ingest.json (2)
1029-1039: LGTM! The new fields enhance the data representation.The addition of fields such as
/title,/description,/charts, and/dashboardUrlimproves the overall data representation for the dashboard configuration.
1050-1067: LGTM! The new field/lastModifiedis well-defined.The addition of the
/lastModifiedfield provides detailed metadata about the creation and modification times, enhancing the data representation.metadata-ingestion/tests/integration/powerbi/golden_test_ingest_patch_disabled.json (40)
3-13: Ensure consistency in aspect data.The
viewPropertiesaspect for thedatasetentity includesmaterialized,viewLogic, andviewLanguage. Ensure that these properties are consistent with the actual dataset configuration in PowerBI.
21-35: Ensure consistency in aspect data.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Verify that these properties are accurate and consistent with the actual dataset configuration in PowerBI.
43-51: Ensure consistency in aspect data.The
statusaspect indicates whether the dataset is removed. Verify that theremovedproperty is accurate and consistent with the actual dataset status in PowerBI.
60-70: Ensure consistency in aspect data.The
subTypesaspect lists the type names for the dataset. Verify that these type names are accurate and consistent with the actual dataset configuration in PowerBI.
79-87: Ensure consistency in aspect data.The
viewPropertiesaspect for thedatasetentity includesmaterialized,viewLogic, andviewLanguage. Ensure that these properties are consistent with the actual dataset configuration in PowerBI.
97-111: Ensure consistency in aspect data.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Verify that these properties are accurate and consistent with the actual dataset configuration in PowerBI.
119-127: Ensure consistency in aspect data.The
statusaspect indicates whether the dataset is removed. Verify that theremovedproperty is accurate and consistent with the actual dataset status in PowerBI.
135-145: Ensure consistency in aspect data.The
subTypesaspect lists the type names for the dataset. Verify that these type names are accurate and consistent with the actual dataset configuration in PowerBI.
154-162: Ensure consistency in aspect data.The
viewPropertiesaspect for thedatasetentity includesmaterialized,viewLogic, andviewLanguage. Ensure that these properties are consistent with the actual dataset configuration in PowerBI.
172-186: Ensure consistency in aspect data.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Verify that these properties are accurate and consistent with the actual dataset configuration in PowerBI.
194-202: Ensure consistency in aspect data.The
statusaspect indicates whether the dataset is removed. Verify that theremovedproperty is accurate and consistent with the actual dataset status in PowerBI.
210-220: Ensure consistency in aspect data.The
subTypesaspect lists the type names for the dataset. Verify that these type names are accurate and consistent with the actual dataset configuration in PowerBI.
229-237: Ensure consistency in aspect data.The
viewPropertiesaspect for thedatasetentity includesmaterialized,viewLogic, andviewLanguage. Ensure that these properties are consistent with the actual dataset configuration in PowerBI.
247-261: Ensure consistency in aspect data.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Verify that these properties are accurate and consistent with the actual dataset configuration in PowerBI.
269-277: Ensure consistency in aspect data.The
statusaspect indicates whether the dataset is removed. Verify that theremovedproperty is accurate and consistent with the actual dataset status in PowerBI.
285-295: Ensure consistency in aspect data.The
subTypesaspect lists the type names for the dataset. Verify that these type names are accurate and consistent with the actual dataset configuration in PowerBI.
303-311: Ensure consistency in aspect data.The
viewPropertiesaspect for thedatasetentity includesmaterialized,viewLogic, andviewLanguage. Ensure that these properties are consistent with the actual dataset configuration in PowerBI.
321-335: Ensure consistency in aspect data.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Verify that these properties are accurate and consistent with the actual dataset configuration in PowerBI.
343-351: Ensure consistency in aspect data.The
statusaspect indicates whether the dataset is removed. Verify that theremovedproperty is accurate and consistent with the actual dataset status in PowerBI.
359-369: Ensure consistency in aspect data.The
subTypesaspect lists the type names for the dataset. Verify that these type names are accurate and consistent with the actual dataset configuration in PowerBI.
378-387: Ensure consistency in aspect data.The
viewPropertiesaspect for thedatasetentity includesmaterialized,viewLogic, andviewLanguage. Ensure that these properties are consistent with the actual dataset configuration in PowerBI.
397-411: Ensure consistency in aspect data.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Verify that these properties are accurate and consistent with the actual dataset configuration in PowerBI.
419-427: Ensure consistency in aspect data.The
statusaspect indicates whether the dataset is removed. Verify that theremovedproperty is accurate and consistent with the actual dataset status in PowerBI.
435-445: Ensure consistency in aspect data.The
subTypesaspect lists the type names for the dataset. Verify that these type names are accurate and consistent with the actual dataset configuration in PowerBI.
453-462: Ensure consistency in aspect data.The
viewPropertiesaspect for thedatasetentity includesmaterialized,viewLogic, andviewLanguage. Ensure that these properties are consistent with the actual dataset configuration in PowerBI.
472-486: Ensure consistency in aspect data.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Verify that these properties are accurate and consistent with the actual dataset configuration in PowerBI.
494-502: Ensure consistency in aspect data.The
statusaspect indicates whether the dataset is removed. Verify that theremovedproperty is accurate and consistent with the actual dataset status in PowerBI.
510-520: Ensure consistency in aspect data.The
subTypesaspect lists the type names for the dataset. Verify that these type names are accurate and consistent with the actual dataset configuration in PowerBI.
528-537: Ensure consistency in aspect data.The
viewPropertiesaspect for thedatasetentity includesmaterialized,viewLogic, andviewLanguage. Ensure that these properties are consistent with the actual dataset configuration in PowerBI.
547-561: Ensure consistency in aspect data.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Verify that these properties are accurate and consistent with the actual dataset configuration in PowerBI.
569-577: Ensure consistency in aspect data.The
statusaspect indicates whether the dataset is removed. Verify that theremovedproperty is accurate and consistent with the actual dataset status in PowerBI.
585-595: Ensure consistency in aspect data.The
subTypesaspect lists the type names for the dataset. Verify that these type names are accurate and consistent with the actual dataset configuration in PowerBI.
603-612: Ensure consistency in aspect data.The
viewPropertiesaspect for thedatasetentity includesmaterialized,viewLogic, andviewLanguage. Ensure that these properties are consistent with the actual dataset configuration in PowerBI.
622-636: Ensure consistency in aspect data.The
datasetPropertiesaspect includescustomProperties,externalUrl,name,description, andtags. Verify that these properties are accurate and consistent with the actual dataset configuration in PowerBI.
644-652: Ensure consistency in aspect data.The
statusaspect indicates whether the dataset is removed. Verify that theremovedproperty is accurate and consistent with the actual dataset status in PowerBI.
660-670: Ensure consistency in aspect data.The
subTypesaspect lists the type names for the dataset. Verify that these type names are accurate and consistent with the actual dataset configuration in PowerBI.
678-686: Ensure consistency in aspect data.The
corpUserKeyaspect for thecorpuserentity includesusername. Ensure that this property is consistent with the actual user configuration in PowerBI.
1123-1130: Ensure consistency in aspect data.The
statusaspect indicates whether the user is removed. Verify that theremovedproperty is accurate and consistent with the actual user status in PowerBI.
710-756: Ensure consistency in aspect data.The
chartInfoaspect for thechartentity includescustomProperties,title,description,lastModified, andinputs. Ensure that these properties are consistent with the actual chart configuration in PowerBI.
766-773: Ensure consistency in aspect data.The
statusaspect indicates whether the chart is removed. Verify that theremovedproperty is accurate and consistent with the actual chart status in PowerBI.metadata-ingestion/tests/integration/powerbi/golden_test_server_to_platform_instance.json (6)
1229-1229: Addition oftitlefield looks good.The
titlefield is correctly added to the JSON structure.
1237-1237: Addition ofdescriptionfield looks good.The
descriptionfield is correctly added to the JSON structure.
1242-1242: Addition ofchartsfield with specific chart URN looks good.The
chartsfield with the specific chart URN is correctly added to the JSON structure.
1247-1247: Addition ofchartsfield with another specific chart URN looks good.The
chartsfield with the specific chart URN is correctly added to the JSON structure.
1252-1252: Addition ofdashboardUrlfield looks good.The
dashboardUrlfield is correctly added to the JSON structure.
1257-1267: Addition oflastModifiedfield looks good.The
lastModifiedfield, including nested objects forcreatedandlastModified, is correctly added to the JSON structure.metadata-ingestion/src/datahub/ingestion/source/powerbi/powerbi.py (3)
1200-1200: Update to logging message looks good.The logging message now accurately references
reportinstead ofdashboard.
1217-1217: Update to return statement looks good.The method now directly returns the mapped
mcpswithout creating an unnecessary variable.
1385-1387: Update to conditional check looks good.The method now includes a reference to
self.source_config.patch_metadata, ensuring the patching operation only occurs if this configuration is enabled.metadata-ingestion/tests/integration/powerbi/test_powerbi.py (9)
686-687: Ensure time consistency in tests.The
@freeze_time(FROZEN_TIME)decorator ensures that the test runs with a consistent time, which is crucial for reproducibility.
688-689: Mock external dependencies.The
@mock.patch("msal.ConfidentialClientApplication", side_effect=mock_msal_cca)decorator mocks the MSAL client to avoid actual API calls, ensuring the test is isolated and does not depend on external services.
690-695: New test functiontest_powerbi_ingest_patch_disabled.The new function tests the PowerBI ingestion pipeline with
patch_metadataset toFalse. It uses various arguments, including mocks and temporary paths, to create a controlled test environment.
696-696: Enable logging for debugging.The
enable_logging()call ensures that logs are captured during the test, which can be helpful for debugging if the test fails.
698-698: Set test resources directory.The
test_resources_dirvariable points to the directory containing test resources, ensuring that the test can access necessary files.
700-700: Register mock API responses.The
register_mock_api(request_mock=requests_mock)call sets up mock responses for API calls, ensuring the test does not depend on actual API responses.
702-718: Create and configure the pipeline.The
Pipeline.createcall creates a pipeline with the specified configuration, including settingpatch_metadatatoFalseand defining the output file path.
721-722: Run the pipeline and handle errors.The
pipeline.run()andpipeline.raise_from_status()calls execute the pipeline and raise any errors encountered during the run, ensuring that the test fails if the pipeline does not complete successfully.
723-729: Verify the output against a golden file.The
mce_helpers.check_golden_filecall compares the pipeline's output to a predefined golden file, ensuring that the output matches expected results.metadata-ingestion/tests/integration/powerbi/golden_test_report.json (6)
1022-1026: Ensure the title field is accurate and descriptive.The
titlefield for the dashboard is set to "test_dashboard". Ensure it accurately reflects the content of the dashboard.
1027-1031: Ensure the description field is comprehensive.The
descriptionfield for the dashboard is set to "Description of test dashboard". Ensure it provides a clear and complete description of the dashboard's purpose and content.
1032-1041: Verify the correctness of chart URNs.The
chartsfield includes URNs for charts. Ensure these URNs are correct and refer to existing charts.
1042-1045: Verify the dashboard URL.The
dashboardUrlfield is set to "https://localhost/dashboards/web/1". Ensure this URL is correct and accessible.
1047-1059: Ensure the lastModified field is accurate.The
lastModifiedfield includes timestamps and actor information. Ensure this information is accurate and up-to-date.
1977-2029: Review the new dashboard entity.A new
dashboardentity has been introduced with fields such astitle,description,charts,dashboardUrl, andlastModified. Ensure all fields are correctly populated and the entity is consistent with other similar entities.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py (4 hunks)
- metadata-ingestion/src/datahub/specific/chart.py (2 hunks)
- metadata-ingestion/src/datahub/specific/dashboard.py (2 hunks)
- metadata-ingestion/tests/integration/powerbi/test_powerbi.py (1 hunks)
- metadata-ingestion/tests/unit/utilities/test_incremental_lineage_helper.py (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- metadata-ingestion/src/datahub/ingestion/api/incremental_lineage_helper.py
- metadata-ingestion/tests/integration/powerbi/test_powerbi.py
Additional comments not posted (21)
metadata-ingestion/src/datahub/specific/chart.py (9)
318-327: LGTM!The method correctly sets the title of a chart and ensures that the title is not
None.
329-338: LGTM!The method correctly sets the description of a chart and ensures that the description is not
None.
340-349: LGTM!The method correctly sets the last refreshed timestamp of a chart if it is provided.
351-362: LGTM!The method correctly sets the last modified details of a chart if they are provided.
364-372: LGTM!The method correctly sets the external URL of a chart if it is provided.
374-383: LGTM!The method correctly sets the chart URL of a chart if it is provided.
385-396: LGTM!The method correctly sets the type of a chart if it is provided.
398-409: LGTM!The method correctly sets the access level of a chart if it is provided.
411-421: LGTM!The method correctly adds input URNs to a chart if they are provided.
metadata-ingestion/src/datahub/specific/dashboard.py (12)
411-420: LGTM!The method correctly sets the title of a dashboard and ensures that the title is not
None.
422-431: LGTM!The method correctly sets the description of a dashboard and ensures that the description is not
None.
433-441: LGTM!The method correctly adds custom properties to a dashboard if they are provided.
443-451: LGTM!The method correctly sets the external URL of a dashboard if it is provided.
453-463: LGTM!The method correctly adds chart URNs to a dashboard if they are provided.
465-477: LGTM!The method correctly adds dataset URNs to a dashboard if they are provided.
479-490: LGTM!The method correctly sets the dashboard URL of a dashboard if it is provided.
492-503: LGTM!The method correctly sets the access level of a dashboard if it is provided.
505-516: LGTM!The method correctly sets the last refreshed timestamp of a dashboard if it is provided.
518-529: LGTM!The method correctly sets the last modified details of a dashboard if they are provided.
433-441: LGTM!The method correctly adds custom properties to a dashboard if they are provided.
433-441: LGTM!The method correctly adds custom properties to a dashboard if they are provided.
| from typing import Optional | ||
|
|
||
| from datahub.ingestion.api.incremental_lineage_helper import convert_chart_info_to_patch | ||
| from datahub.ingestion.api.workunit import MetadataWorkUnit | ||
| from datahub.metadata.schema_classes import ( | ||
| ChangeAuditStampsClass, | ||
| ChartInfoClass, | ||
| MetadataChangeProposalClass, | ||
| ) | ||
|
|
||
|
|
||
| def test_convert_chart_info_to_patch(): | ||
| chart_info_class: ChartInfoClass = ChartInfoClass( | ||
| title="foo", | ||
| description="Checking patch", | ||
| inputs=[ | ||
| "urn:li:dataset:(urn:li:dataPlatform:dbt,long_tail_companions.analytics.pet_details,PROD)", | ||
| "urn:li:dataset:(urn:li:dataPlatform:dbt,calm-pagoda-323403.jaffle_shop.customers,PROD)", | ||
| ], | ||
| lastModified=ChangeAuditStampsClass(), | ||
| ) | ||
|
|
||
| mw: Optional[MetadataWorkUnit] = convert_chart_info_to_patch( | ||
| urn="urn:li:chart:(looker,dashboard_elements.1)", | ||
| aspect=chart_info_class, | ||
| system_metadata=None, | ||
| ) | ||
|
|
||
| assert mw | ||
|
|
||
| assert mw.id == "urn:li:chart:(looker,dashboard_elements.1)-chartInfo" | ||
|
|
||
| assert isinstance(mw.metadata, MetadataChangeProposalClass) | ||
|
|
||
| assert mw.metadata.aspect | ||
|
|
||
| assert ( | ||
| mw.metadata.aspect.value | ||
| == b'[{"op": "add", "path": "/title", "value": "foo"}, {"op": "add", "path": "/lastModified", "value": {"created": {"time": 0, "actor": "urn:li:corpuser:unknown"}, "lastModified": {"time": 0, "actor": "urn:li:corpuser:unknown"}}}, {"op": "add", "path": "/description", "value": "Checking patch"}, {"op": "add", "path": "/inputs/urn:li:dataset:(urn:li:dataPlatform:dbt,long_tail_companions.analytics.pet_details,PROD)", "value": "urn:li:dataset:(urn:li:dataPlatform:dbt,long_tail_companions.analytics.pet_details,PROD)"}, {"op": "add", "path": "/inputs/urn:li:dataset:(urn:li:dataPlatform:dbt,calm-pagoda-323403.jaffle_shop.customers,PROD)", "value": "urn:li:dataset:(urn:li:dataPlatform:dbt,calm-pagoda-323403.jaffle_shop.customers,PROD)"}]' | ||
| ) |
There was a problem hiding this comment.
LGTM! Consider adding more assertions.
The test case is well-structured and covers the basic functionality. However, consider adding more assertions to cover edge cases and different scenarios, such as:
- When
inputsis empty. - When
system_metadatais notNone. - When
lastModifiedhas specific values.
* feat(forms) Handle deleting forms references when hard deleting forms (datahub-project#10820) * refactor(ui): Misc improvements to the setup ingestion flow (ingest uplift 1/2) (datahub-project#10764) Co-authored-by: John Joyce <[email protected]> Co-authored-by: John Joyce <[email protected]> * fix(ingestion/airflow-plugin): pipeline tasks discoverable in search (datahub-project#10819) * feat(ingest/transformer): tags to terms transformer (datahub-project#10758) Co-authored-by: Aseem Bansal <[email protected]> * fix(ingestion/unity-catalog): fixed issue with profiling with GE turned on (datahub-project#10752) Co-authored-by: Aseem Bansal <[email protected]> * feat(forms) Add java SDK for form entity PATCH + CRUD examples (datahub-project#10822) * feat(SDK) Add java SDK for structuredProperty entity PATCH + CRUD examples (datahub-project#10823) * feat(SDK) Add StructuredPropertyPatchBuilder in python sdk and provide sample CRUD files (datahub-project#10824) * feat(forms) Add CRUD endpoints to GraphQL for Form entities (datahub-project#10825) * add flag for includeSoftDeleted in scroll entities API (datahub-project#10831) * feat(deprecation) Return actor entity with deprecation aspect (datahub-project#10832) * feat(structuredProperties) Add CRUD graphql APIs for structured property entities (datahub-project#10826) * add scroll parameters to openapi v3 spec (datahub-project#10833) * fix(ingest): correct profile_day_of_week implementation (datahub-project#10818) * feat(ingest/glue): allow ingestion of empty databases from Glue (datahub-project#10666) Co-authored-by: Harshal Sheth <[email protected]> * feat(cli): add more details to get cli (datahub-project#10815) * fix(ingestion/glue): ensure date formatting works on all platforms for aws glue (datahub-project#10836) * fix(ingestion): fix datajob patcher (datahub-project#10827) * fix(smoke-test): add suffix in temp file creation (datahub-project#10841) * feat(ingest/glue): add helper method to permit user or group ownership (datahub-project#10784) * feat(): Show data platform instances in policy modal if they are set on the policy (datahub-project#10645) Co-authored-by: Hendrik Richert <[email protected]> * docs(patch): add patch documentation for how implementation works (datahub-project#10010) Co-authored-by: John Joyce <[email protected]> * fix(jar): add missing custom-plugin-jar task (datahub-project#10847) * fix(): also check exceptions/stack trace when filtering log messages (datahub-project#10391) Co-authored-by: John Joyce <[email protected]> * docs(): Update posts.md (datahub-project#9893) Co-authored-by: Hyejin Yoon <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore(ingest): update acryl-datahub-classify version (datahub-project#10844) * refactor(ingest): Refactor structured logging to support infos, warnings, and failures structured reporting to UI (datahub-project#10828) Co-authored-by: John Joyce <[email protected]> Co-authored-by: Harshal Sheth <[email protected]> * fix(restli): log aspect-not-found as a warning rather than as an error (datahub-project#10834) * fix(ingest/nifi): remove duplicate upstream jobs (datahub-project#10849) * fix(smoke-test): test access to create/revoke personal access tokens (datahub-project#10848) * fix(smoke-test): missing test for move domain (datahub-project#10837) * ci: update usernames to not considered for community (datahub-project#10851) * env: change defaults for data contract visibility (datahub-project#10854) * fix(ingest/tableau): quote special characters in external URL (datahub-project#10842) * fix(smoke-test): fix flakiness of auto complete test * ci(ingest): pin dask dependency for feast (datahub-project#10865) * fix(ingestion/lookml): liquid template resolution and view-to-view cll (datahub-project#10542) * feat(ingest/audit): add client id and version in system metadata props (datahub-project#10829) * chore(ingest): Mypy 1.10.1 pin (datahub-project#10867) * docs: use acryl-datahub-actions as expected python package to install (datahub-project#10852) * docs: add new js snippet (datahub-project#10846) * refactor(ingestion): remove company domain for security reason (datahub-project#10839) * fix(ingestion/spark): Platform instance and column level lineage fix (datahub-project#10843) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(ingestion/tableau): optionally ingest multiple sites and create site containers (datahub-project#10498) Co-authored-by: Yanik Häni <[email protected]> * fix(ingestion/looker): Add sqlglot dependency and remove unused sqlparser (datahub-project#10874) * fix(manage-tokens): fix manage access token policy (datahub-project#10853) * Batch get entity endpoints (datahub-project#10880) * feat(system): support conditional write semantics (datahub-project#10868) * fix(build): upgrade vercel builds to Node 20.x (datahub-project#10890) * feat(ingest/lookml): shallow clone repos (datahub-project#10888) * fix(ingest/looker): add missing dependency (datahub-project#10876) * fix(ingest): only populate audit stamps where accurate (datahub-project#10604) * fix(ingest/dbt): always encode tag urns (datahub-project#10799) * fix(ingest/redshift): handle multiline alter table commands (datahub-project#10727) * fix(ingestion/looker): column name missing in explore (datahub-project#10892) * fix(lineage) Fix lineage source/dest filtering with explored per hop limit (datahub-project#10879) * feat(conditional-writes): misc updates and fixes (datahub-project#10901) * feat(ci): update outdated action (datahub-project#10899) * feat(rest-emitter): adding async flag to rest emitter (datahub-project#10902) Co-authored-by: Gabe Lyons <[email protected]> * feat(ingest): add snowflake-queries source (datahub-project#10835) * fix(ingest): improve `auto_materialize_referenced_tags_terms` error handling (datahub-project#10906) * docs: add new company to adoption list (datahub-project#10909) * refactor(redshift): Improve redshift error handling with new structured reporting system (datahub-project#10870) Co-authored-by: John Joyce <[email protected]> Co-authored-by: Harshal Sheth <[email protected]> * feat(ui) Finalize support for all entity types on forms (datahub-project#10915) * Index ExecutionRequestResults status field (datahub-project#10811) * feat(ingest): grafana connector (datahub-project#10891) Co-authored-by: Shirshanka Das <[email protected]> Co-authored-by: Harshal Sheth <[email protected]> * fix(gms) Add Form entity type to EntityTypeMapper (datahub-project#10916) * feat(dataset): add support for external url in Dataset (datahub-project#10877) * docs(saas-overview) added missing features to observe section (datahub-project#10913) Co-authored-by: John Joyce <[email protected]> * fix(ingest/spark): Fixing Micrometer warning (datahub-project#10882) * fix(structured properties): allow application of structured properties without schema file (datahub-project#10918) * fix(data-contracts-web) handle other schedule types (datahub-project#10919) * fix(ingestion/tableau): human-readable message for PERMISSIONS_MODE_SWITCHED error (datahub-project#10866) Co-authored-by: Harshal Sheth <[email protected]> * Add feature flag for view defintions (datahub-project#10914) Co-authored-by: Ethan Cartwright <[email protected]> * feat(ingest/BigQuery): refactor+parallelize dataset metadata extraction (datahub-project#10884) * fix(airflow): add error handling around render_template() (datahub-project#10907) * feat(ingestion/sqlglot): add optional `default_dialect` parameter to sqlglot lineage (datahub-project#10830) * feat(mcp-mutator): new mcp mutator plugin (datahub-project#10904) * fix(ingest/bigquery): changes helper function to decode unicode scape sequences (datahub-project#10845) * feat(ingest/postgres): fetch table sizes for profile (datahub-project#10864) * feat(ingest/abs): Adding azure blob storage ingestion source (datahub-project#10813) * fix(ingest/redshift): reduce severity of SQL parsing issues (datahub-project#10924) * fix(build): fix lint fix web react (datahub-project#10896) * fix(ingest/bigquery): handle quota exceeded for project.list requests (datahub-project#10912) * feat(ingest): report extractor failures more loudly (datahub-project#10908) * feat(ingest/snowflake): integrate snowflake-queries into main source (datahub-project#10905) * fix(ingest): fix docs build (datahub-project#10926) * fix(ingest/snowflake): fix test connection (datahub-project#10927) * fix(ingest/lookml): add view load failures to cache (datahub-project#10923) * docs(slack) overhauled setup instructions and screenshots (datahub-project#10922) Co-authored-by: John Joyce <[email protected]> * fix(airflow): Add comma parsing of owners to DataJobs (datahub-project#10903) * fix(entityservice): fix merging sideeffects (datahub-project#10937) * feat(ingest): Support System Ingestion Sources, Show and hide system ingestion sources with Command-S (datahub-project#10938) Co-authored-by: John Joyce <[email protected]> * chore() Set a default lineage filtering end time on backend when a start time is present (datahub-project#10925) Co-authored-by: John Joyce <[email protected]> Co-authored-by: John Joyce <[email protected]> * Added relationships APIs to V3. Added these generic APIs to V3 swagger doc. (datahub-project#10939) * docs: add learning center to docs (datahub-project#10921) * doc: Update hubspot form id (datahub-project#10943) * chore(airflow): add python 3.11 w/ Airflow 2.9 to CI (datahub-project#10941) * fix(ingest/Glue): column upstream lineage between S3 and Glue (datahub-project#10895) * fix(ingest/abs): split abs utils into multiple files (datahub-project#10945) * doc(ingest/looker): fix doc for sql parsing documentation (datahub-project#10883) Co-authored-by: Harshal Sheth <[email protected]> * fix(ingest/bigquery): Adding missing BigQuery types (datahub-project#10950) * fix(ingest/setup): feast and abs source setup (datahub-project#10951) * fix(connections) Harden adding /gms to connections in backend (datahub-project#10942) * feat(siblings) Add flag to prevent combining siblings in the UI (datahub-project#10952) * fix(docs): make graphql doc gen more automated (datahub-project#10953) * feat(ingest/athena): Add option for Athena partitioned profiling (datahub-project#10723) * fix(spark-lineage): default timeout for future responses (datahub-project#10947) * feat(datajob/flow): add environment filter using info aspects (datahub-project#10814) * fix(ui/ingest): correct privilege used to show tab (datahub-project#10483) Co-authored-by: Kunal-kankriya <[email protected]> * feat(ingest/looker): include dashboard urns in browse v2 (datahub-project#10955) * add a structured type to batchGet in OpenAPI V3 spec (datahub-project#10956) * fix(ui): scroll on the domain sidebar to show all domains (datahub-project#10966) * fix(ingest/sagemaker): resolve incorrect variable assignment for SageMaker API call (datahub-project#10965) * fix(airflow/build): Pinning mypy (datahub-project#10972) * Fixed a bug where the OpenAPI V3 spec was incorrect. The bug was introduced in datahub-project#10939. (datahub-project#10974) * fix(ingest/test): Fix for mssql integration tests (datahub-project#10978) * fix(entity-service) exist check correctly extracts status (datahub-project#10973) * fix(structuredProps) casing bug in StructuredPropertiesValidator (datahub-project#10982) * bugfix: use anyOf instead of allOf when creating references in openapi v3 spec (datahub-project#10986) * fix(ui): Remove ant less imports (datahub-project#10988) * feat(ingest/graph): Add get_results_by_filter to DataHubGraph (datahub-project#10987) * feat(ingest/cli): init does not actually support environment variables (datahub-project#10989) * fix(ingest/graph): Update get_results_by_filter graphql query (datahub-project#10991) * feat(ingest/spark): Promote beta plugin (datahub-project#10881) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(ingest): support domains in meta -> "datahub" section (datahub-project#10967) * feat(ingest): add `check server-config` command (datahub-project#10990) * feat(cli): Make consistent use of DataHubGraphClientConfig (datahub-project#10466) Deprecates get_url_and_token() in favor of a more complete option: load_graph_config() that returns a full DatahubClientConfig. This change was then propagated across previous usages of get_url_and_token so that connections to DataHub server from the client respect the full breadth of configuration specified by DatahubClientConfig. I.e: You can now specify disable_ssl_verification: true in your ~/.datahubenv file so that all cli functions to the server work when ssl certification is disabled. Fixes datahub-project#9705 * fix(ingest/s3): Fixing container creation when there is no folder in path (datahub-project#10993) * fix(ingest/looker): support platform instance for dashboards & charts (datahub-project#10771) * feat(ingest/bigquery): improve handling of information schema in sql parser (datahub-project#10985) * feat(ingest): improve `ingest deploy` command (datahub-project#10944) * fix(backend): allow excluding soft-deleted entities in relationship-queries; exclude soft-deleted members of groups (datahub-project#10920) - allow excluding soft-deleted entities in relationship-queries - exclude soft-deleted members of groups * fix(ingest/looker): downgrade missing chart type log level (datahub-project#10996) * doc(acryl-cloud): release docs for 0.3.4.x (datahub-project#10984) Co-authored-by: John Joyce <[email protected]> Co-authored-by: RyanHolstien <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Pedro Silva <[email protected]> * fix(protobuf/build): Fix protobuf check jar script (datahub-project#11006) * fix(ui/ingest): Support invalid cron jobs (datahub-project#10998) * fix(ingest): fix graph config loading (datahub-project#11002) Co-authored-by: Pedro Silva <[email protected]> * feat(docs): Document __DATAHUB_TO_FILE_ directive (datahub-project#10968) Co-authored-by: Harshal Sheth <[email protected]> * fix(graphql/upsertIngestionSource): Validate cron schedule; parse error in CLI (datahub-project#11011) * feat(ece): support custom ownership type urns in ECE generation (datahub-project#10999) * feat(assertion-v2): changed Validation tab to Quality and created new Governance tab (datahub-project#10935) * fix(ingestion/glue): Add support for missing config options for profiling in Glue (datahub-project#10858) * feat(propagation): Add models for schema field docs, tags, terms (datahub-project#2959) (datahub-project#11016) Co-authored-by: Chris Collins <[email protected]> * docs: standardize terminology to DataHub Cloud (datahub-project#11003) * fix(ingestion/transformer): replace the externalUrl container (datahub-project#11013) * docs(slack) troubleshoot docs (datahub-project#11014) * feat(propagation): Add graphql API (datahub-project#11030) Co-authored-by: Chris Collins <[email protected]> * feat(propagation): Add models for Action feature settings (datahub-project#11029) * docs(custom properties): Remove duplicate from sidebar (datahub-project#11033) * feat(models): Introducing Dataset Partitions Aspect (datahub-project#10997) Co-authored-by: John Joyce <[email protected]> Co-authored-by: John Joyce <[email protected]> * feat(propagation): Add Documentation Propagation Settings (datahub-project#11038) * fix(models): chart schema fields mapping, add dataHubAction entity, t… (datahub-project#11040) * fix(ci): smoke test lint failures (datahub-project#11044) * docs: fix learning center color scheme & typo (datahub-project#11043) * feat: add cloud main page (datahub-project#11017) Co-authored-by: Jay <[email protected]> * feat(restore-indices): add additional step to also clear system metadata service (datahub-project#10662) Co-authored-by: John Joyce <[email protected]> * docs: fix typo (datahub-project#11046) * fix(lint): apply spotless (datahub-project#11050) * docs(airflow): example query to get datajobs for a dataflow (datahub-project#11034) * feat(cli): Add run-id option to put sub-command (datahub-project#11023) Adds an option to assign run-id to a given put command execution. This is useful when transformers do not exist for a given ingestion payload, we can follow up with custom metadata and assign it to an ingestion pipeline. * fix(ingest): improve sql error reporting calls (datahub-project#11025) * fix(airflow): fix CI setup (datahub-project#11031) * feat(ingest/dbt): add experimental `prefer_sql_parser_lineage` flag (datahub-project#11039) * fix(ingestion/lookml): enable stack-trace in lookml logs (datahub-project#10971) * (chore): Linting fix (datahub-project#11015) * chore(ci): update deprecated github actions (datahub-project#10977) * Fix ALB configuration example (datahub-project#10981) * chore(ingestion-base): bump base image packages (datahub-project#11053) * feat(cli): Trim report of dataHubExecutionRequestResult to max GMS size (datahub-project#11051) * fix(ingestion/lookml): emit dummy sql condition for lookml custom condition tag (datahub-project#11008) Co-authored-by: Harshal Sheth <[email protected]> * fix(ingestion/powerbi): fix issue with broken report lineage (datahub-project#10910) * feat(ingest/tableau): add retry on timeout (datahub-project#10995) * change generate kafka connect properties from env (datahub-project#10545) Co-authored-by: david-leifker <[email protected]> * fix(ingest): fix oracle cronjob ingestion (datahub-project#11001) Co-authored-by: david-leifker <[email protected]> * chore(ci): revert update deprecated github actions (datahub-project#10977) (datahub-project#11062) * feat(ingest/dbt-cloud): update metadata_endpoint inference (datahub-project#11041) * build: Reduce size of datahub-frontend-react image by 50-ish% (datahub-project#10878) Co-authored-by: david-leifker <[email protected]> * fix(ci): Fix lint issue in datahub_ingestion_run_summary_provider.py (datahub-project#11063) * docs(ingest): update developing-a-transformer.md (datahub-project#11019) * feat(search-test): update search tests from datahub-project#10408 (datahub-project#11056) * feat(cli): add aspects parameter to DataHubGraph.get_entity_semityped (datahub-project#11009) Co-authored-by: Harshal Sheth <[email protected]> * docs(airflow): update min version for plugin v2 (datahub-project#11065) * doc(ingestion/tableau): doc update for derived permission (datahub-project#11054) Co-authored-by: Pedro Silva <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Harshal Sheth <[email protected]> * fix(py): remove dep on types-pkg_resources (datahub-project#11076) * feat(ingest/mode): add option to exclude restricted (datahub-project#11081) * fix(ingest): set lastObserved in sdk when unset (datahub-project#11071) * doc(ingest): Update capabilities (datahub-project#11072) * chore(vulnerability): Log Injection (datahub-project#11090) * chore(vulnerability): Information exposure through a stack trace (datahub-project#11091) * chore(vulnerability): Comparison of narrow type with wide type in loop condition (datahub-project#11089) * chore(vulnerability): Insertion of sensitive information into log files (datahub-project#11088) * chore(vulnerability): Risky Cryptographic Algorithm (datahub-project#11059) * chore(vulnerability): Overly permissive regex range (datahub-project#11061) Co-authored-by: Harshal Sheth <[email protected]> * fix: update customer data (datahub-project#11075) * fix(models): fixing the datasetPartition models (datahub-project#11085) Co-authored-by: John Joyce <[email protected]> * fix(ui): Adding view, forms GraphQL query, remove showing a fallback error message on unhandled GraphQL error (datahub-project#11084) Co-authored-by: John Joyce <[email protected]> * feat(docs-site): hiding learn more from cloud page (datahub-project#11097) * fix(docs): Add correct usage of orFilters in search API docs (datahub-project#11082) Co-authored-by: Jay <[email protected]> * fix(ingest/mode): Regexp in mode name matcher didn't allow underscore (datahub-project#11098) * docs: Refactor customer stories section (datahub-project#10869) Co-authored-by: Jeff Merrick <[email protected]> * fix(release): fix full/slim suffix on tag (datahub-project#11087) * feat(config): support alternate hashing algorithm for doc id (datahub-project#10423) Co-authored-by: david-leifker <[email protected]> Co-authored-by: John Joyce <[email protected]> * fix(emitter): fix typo in get method of java kafka emitter (datahub-project#11007) * fix(ingest): use correct native data type in all SQLAlchemy sources by compiling data type using dialect (datahub-project#10898) Co-authored-by: Harshal Sheth <[email protected]> * chore: Update contributors list in PR labeler (datahub-project#11105) * feat(ingest): tweak stale entity removal messaging (datahub-project#11064) * fix(ingestion): enforce lastObserved timestamps in SystemMetadata (datahub-project#11104) * fix(ingest/powerbi): fix broken lineage between chart and dataset (datahub-project#11080) * feat(ingest/lookml): CLL support for sql set in sql_table_name attribute of lookml view (datahub-project#11069) * docs: update graphql docs on forms & structured properties (datahub-project#11100) * test(search): search openAPI v3 test (datahub-project#11049) * fix(ingest/tableau): prevent empty site content urls (datahub-project#11057) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(entity-client): implement client batch interface (datahub-project#11106) * fix(snowflake): avoid reporting warnings/info for sys tables (datahub-project#11114) * fix(ingest): downgrade column type mapping warning to info (datahub-project#11115) * feat(api): add AuditStamp to the V3 API entity/aspect response (datahub-project#11118) * fix(ingest/redshift): replace r'\n' with '\n' to avoid token error redshift serverless… (datahub-project#11111) * fix(entiy-client): handle null entityUrn case for restli (datahub-project#11122) * fix(sql-parser): prevent bad urns from alter table lineage (datahub-project#11092) * fix(ingest/bigquery): use small batch size if use_tables_list_query_v2 is set (datahub-project#11121) * fix(graphql): add missing entities to EntityTypeMapper and EntityTypeUrnMapper (datahub-project#10366) * feat(ui): Changes to allow editable dataset name (datahub-project#10608) Co-authored-by: Jay Kadambi <[email protected]> * fix: remove saxo (datahub-project#11127) * feat(mcl-processor): Update mcl processor hooks (datahub-project#11134) * fix(openapi): fix openapi v2 endpoints & v3 documentation update * Revert "fix(openapi): fix openapi v2 endpoints & v3 documentation update" This reverts commit 573c1cb. * docs(policies): updates to policies documentation (datahub-project#11073) * fix(openapi): fix openapi v2 and v3 docs update (datahub-project#11139) * feat(auth): grant type and acr values custom oidc parameters support (datahub-project#11116) * fix(mutator): mutator hook fixes (datahub-project#11140) * feat(search): support sorting on multiple fields (datahub-project#10775) * feat(ingest): various logging improvements (datahub-project#11126) * fix(ingestion/lookml): fix for sql parsing error (datahub-project#11079) Co-authored-by: Harshal Sheth <[email protected]> * feat(docs-site) cloud page spacing and content polishes (datahub-project#11141) * feat(ui) Enable editing structured props on fields (datahub-project#11042) * feat(tests): add md5 and last computed to testResult model (datahub-project#11117) * test(openapi): openapi regression smoke tests (datahub-project#11143) * fix(airflow): fix tox tests + update docs (datahub-project#11125) * docs: add chime to adoption stories (datahub-project#11142) * fix(ingest/databricks): Updating code to work with Databricks sdk 0.30 (datahub-project#11158) * fix(kafka-setup): add missing script to image (datahub-project#11190) * fix(config): fix hash algo config (datahub-project#11191) * test(smoke-test): updates to smoke-tests (datahub-project#11152) * fix(elasticsearch): refactor idHashAlgo setting (datahub-project#11193) * chore(kafka): kafka version bump (datahub-project#11211) * readd UsageStatsWorkUnit * fix merge problems * change logo --------- Co-authored-by: Chris Collins <[email protected]> Co-authored-by: John Joyce <[email protected]> Co-authored-by: John Joyce <[email protected]> Co-authored-by: John Joyce <[email protected]> Co-authored-by: dushayntAW <[email protected]> Co-authored-by: sagar-salvi-apptware <[email protected]> Co-authored-by: Aseem Bansal <[email protected]> Co-authored-by: Kevin Chun <[email protected]> Co-authored-by: jordanjeremy <[email protected]> Co-authored-by: skrydal <[email protected]> Co-authored-by: Harshal Sheth <[email protected]> Co-authored-by: david-leifker <[email protected]> Co-authored-by: sid-acryl <[email protected]> Co-authored-by: Julien Jehannet <[email protected]> Co-authored-by: Hendrik Richert <[email protected]> Co-authored-by: Hendrik Richert <[email protected]> Co-authored-by: RyanHolstien <[email protected]> Co-authored-by: Felix Lüdin <[email protected]> Co-authored-by: Pirry <[email protected]> Co-authored-by: Hyejin Yoon <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: cburroughs <[email protected]> Co-authored-by: ksrinath <[email protected]> Co-authored-by: Mayuri Nehate <[email protected]> Co-authored-by: Kunal-kankriya <[email protected]> Co-authored-by: Shirshanka Das <[email protected]> Co-authored-by: ipolding-cais <[email protected]> Co-authored-by: Tamas Nemeth <[email protected]> Co-authored-by: Shubham Jagtap <[email protected]> Co-authored-by: haeniya <[email protected]> Co-authored-by: Yanik Häni <[email protected]> Co-authored-by: Gabe Lyons <[email protected]> Co-authored-by: Gabe Lyons <[email protected]> Co-authored-by: 808OVADOZE <[email protected]> Co-authored-by: noggi <[email protected]> Co-authored-by: Nicholas Pena <[email protected]> Co-authored-by: Jay <[email protected]> Co-authored-by: ethan-cartwright <[email protected]> Co-authored-by: Ethan Cartwright <[email protected]> Co-authored-by: Nadav Gross <[email protected]> Co-authored-by: Patrick Franco Braz <[email protected]> Co-authored-by: pie1nthesky <[email protected]> Co-authored-by: Joel Pinto Mata (KPN-DSH-DEX team) <[email protected]> Co-authored-by: Ellie O'Neil <[email protected]> Co-authored-by: Ajoy Majumdar <[email protected]> Co-authored-by: deepgarg-visa <[email protected]> Co-authored-by: Tristan Heisler <[email protected]> Co-authored-by: Andrew Sikowitz <[email protected]> Co-authored-by: Davi Arnaut <[email protected]> Co-authored-by: Pedro Silva <[email protected]> Co-authored-by: amit-apptware <[email protected]> Co-authored-by: Sam Black <[email protected]> Co-authored-by: Raj Tekal <[email protected]> Co-authored-by: Steffen Grohsschmiedt <[email protected]> Co-authored-by: jaegwon.seo <[email protected]> Co-authored-by: Renan F. Lima <[email protected]> Co-authored-by: Matt Exchange <[email protected]> Co-authored-by: Jonny Dixon <[email protected]> Co-authored-by: Pedro Silva <[email protected]> Co-authored-by: Pinaki Bhattacharjee <[email protected]> Co-authored-by: Jeff Merrick <[email protected]> Co-authored-by: skrydal <[email protected]> Co-authored-by: AndreasHegerNuritas <[email protected]> Co-authored-by: jayasimhankv <[email protected]> Co-authored-by: Jay Kadambi <[email protected]> Co-authored-by: David Leifker <[email protected]>
Summary by CodeRabbit
New Features
PatchEntityAspect,DashboardPatchBuilder, andChartPatchBuilder, improving metadata updates.convert_dashboard_info_to_patchfunction with added attribute settings and a logging mechanism for better patch metadata generation.Bug Fixes
Refactor
McpPatchBuilder.Tests
patch_metadatais disabled.convert_chart_info_to_patchfunction to ensure correct transformation of chart metadata.