[DO NOT MERGE] - Sap hana improvements #11133
Conversation
WalkthroughThe recent changes to 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- metadata-ingestion/src/datahub/ingestion/source/sql/hana.py (3 hunks)
Additional context used
Ruff
metadata-ingestion/src/datahub/ingestion/source/sql/hana.py
242-245: f-string without any placeholders
Remove extraneous
fprefix(F541)
250-256: f-string without any placeholders
Remove extraneous
fprefix(F541)
Additional comments not posted (16)
metadata-ingestion/src/datahub/ingestion/source/sql/hana.py (16)
1-45: Imports look good.The imports are relevant to the new functionalities introduced in the file.
62-94: Type mapping is well-defined.The
HANA_TYPES_MAPprovides a comprehensive mapping of HANA data types to DataHub schema classes.
97-99: Preprocessing function is effective.The
preprocess_sap_typefunction effectively removes parameters from SAP type strings using regex.
102-104: Type conversion function is efficient.The
get_pegasus_typefunction efficiently converts SAP types to DataHub schema classes using the preprocessing and mapping.
107-115: Configuration class is well-structured.The
BaseHanaConfigclass appropriately extendsBasicSQLAlchemyConfigwith HANA-specific configurations.
118-129: Extended configuration class is appropriate.The
HanaConfigclass extendsBaseHanaConfigwith additional fields for database filtering, enhancing flexibility.
143-159: Constructor setup is correct.The
__init__method correctly initializes theSqlParsingAggregatorand sets up the SQL engine.
169-171: Engine creation method is straightforward.The
_create_enginemethod is essential for establishing database connectivity.
173-175: Table properties retrieval is valuable.The
get_table_propertiesmethod effectively retrieves table metadata, aiding in documentation and understanding.
177-187: View properties retrieval is efficient.The
get_view_propertiesmethod efficiently fetches view details necessary for metadata ingestion.
189-199: View info organization is effective.The
get_view_infomethod organizes view metadata into aDatasetPropertiesClass, aiding in structured data management.
201-208: Lineage retrieval is crucial.The
get_lineagemethod efficiently retrieves lineage information, which is essential for understanding data dependencies.
210-227: Lineage construction is well-structured.The
construct_lineagemethod organizes upstream data into anUpstreamLineageClass, aiding in lineage management.
229-239: Column metadata retrieval is efficient.The
get_columnsmethod efficiently gathers column metadata, which is crucial for schema management.
467-501: Calculation view lineage extraction is effective.The
get_calculation_view_lineagemethod effectively extracts lineage information from XML, aiding in understanding calculation view dependencies.
505-572: Calculation view processing is comprehensive.The
_process_calculation_viewmethod effectively handles the ingestion and processing of calculation view metadata.
| def get_query_logs(self) -> List[Dict]: | ||
| query = f""" | ||
| SELECT STATEMENT_STRING, | ||
| USER_NAME, LAST_EXECUTION_TIMESTAMP | ||
| FROM SYS.M_SQL_PLAN_CACHE""" |
There was a problem hiding this comment.
Remove unnecessary f-string.
The query string does not require an f-string since there are no placeholders.
- query = f"""
+ query = """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 get_query_logs(self) -> List[Dict]: | |
| query = f""" | |
| SELECT STATEMENT_STRING, | |
| USER_NAME, LAST_EXECUTION_TIMESTAMP | |
| FROM SYS.M_SQL_PLAN_CACHE""" | |
| def get_query_logs(self) -> List[Dict]: | |
| query = """ | |
| SELECT STATEMENT_STRING, | |
| USER_NAME, LAST_EXECUTION_TIMESTAMP | |
| FROM SYS.M_SQL_PLAN_CACHE""" |
Tools
Ruff
242-245: f-string without any placeholders
Remove extraneous
fprefix(F541)
| def get_package_names(self) -> List[dict]: | ||
| query = f""" | ||
| SELECT | ||
| PACKAGE_ID, | ||
| OBJECT_NAME, | ||
| CDATA | ||
| FROM _SYS_REPO.ACTIVE_OBJECT | ||
| WHERE OBJECT_SUFFIX='calculationview'""" |
There was a problem hiding this comment.
Remove unnecessary f-string.
The query string does not require an f-string since there are no placeholders.
- query = f"""
+ query = """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 get_package_names(self) -> List[dict]: | |
| query = f""" | |
| SELECT | |
| PACKAGE_ID, | |
| OBJECT_NAME, | |
| CDATA | |
| FROM _SYS_REPO.ACTIVE_OBJECT | |
| WHERE OBJECT_SUFFIX='calculationview'""" | |
| def get_package_names(self) -> List[dict]: | |
| query = """ | |
| SELECT | |
| PACKAGE_ID, | |
| OBJECT_NAME, | |
| CDATA | |
| FROM _SYS_REPO.ACTIVE_OBJECT | |
| WHERE OBJECT_SUFFIX='calculationview'""" |
Tools
Ruff
250-256: f-string without any placeholders
Remove extraneous
fprefix(F541)
acrylJonny
changed the title
|
@acrylJonny marking this as a draft PR until it's ready for review |
Checklist
Summary by CodeRabbit
New Features
Bug Fixes