diff --git a/ingestion/src/metadata/ingestion/source/database/saperp/client.py b/ingestion/src/metadata/ingestion/source/database/saperp/client.py index a7fa52ad382..608e85bd027 100644 --- a/ingestion/src/metadata/ingestion/source/database/saperp/client.py +++ b/ingestion/src/metadata/ingestion/source/database/saperp/client.py @@ -13,7 +13,6 @@ Client to interact with SAP ERP APIs """ import math -import time import traceback from typing import Any, List, Optional, Union @@ -21,6 +20,7 @@ from metadata.generated.schema.entity.services.connections.database.sapErpConnec SapErpConnection, ) from metadata.ingestion.ometa.client import REST, ClientConfig +from metadata.ingestion.source.database.saperp.constants import PARAMS_DATA from metadata.ingestion.source.database.saperp.models import ( SapErpColumn, SapErpColumnResponse, @@ -29,6 +29,7 @@ from metadata.ingestion.source.database.saperp.models import ( ) from metadata.utils.helpers import clean_uri from metadata.utils.logger import ingestion_logger +from metadata.utils.ssl_registry import get_verify_ssl_fn logger = ingestion_logger() @@ -49,14 +50,17 @@ class SapErpClient: def __init__(self, config: SapErpConnection): self.config: SapErpConnection = config self.auth_token = self.config.apiKey.get_secret_value() + get_verify_ssl = get_verify_ssl_fn(config.verifySSL) client_config: ClientConfig = ClientConfig( base_url=clean_uri(config.hostPort), auth_header="APIKey", auth_token_mode="", auth_token=lambda: (self.auth_token, 0), api_version="v1", - verify=False, allow_redirects=True, + retry_codes=[500, 504], + retry_wait=5, + verify=get_verify_ssl(config.sslConfig), ) self.client = REST(client_config) @@ -64,7 +68,7 @@ class SapErpClient: """ Check metadata connection to SAS ERP tables API """ - params_data = {"$top": "1", "$format": "json", "$inlinecount": "allpages"} + params_data = PARAMS_DATA response_data = self.client._request( # pylint: disable=protected-access method="GET", path="/ECC/DDIC/ZZ_I_DDIC_TAB_CDS/", @@ -81,7 +85,7 @@ class SapErpClient: """ Check metadata connection to SAP ERP columns API """ - params_data = {"$top": "1", "$format": "json", "$inlinecount": "allpages"} + params_data = PARAMS_DATA response_data = self.client._request( # pylint: disable=protected-access method="GET", path="/ECC/DDIC/ZZ_I_DDIC_COL_CDS/", @@ -101,7 +105,7 @@ class SapErpClient: Method to paginate the APIs """ entities_list = [] - params_data.update({"$top": "1", "$format": "json", "$inlinecount": "allpages"}) + params_data.update(PARAMS_DATA) response_data = self.client._request( # pylint: disable=protected-access method="GET", path=api_url, headers=HEADERS, data=params_data ) @@ -123,9 +127,6 @@ class SapErpClient: ) response = model_class(**response_data) entities_list.extend(response.d.results) - # Adding a delay before sending the requests to server - # due to server throwing error when too many requests are sent - time.sleep(0.5) except Exception as exc: logger.debug(traceback.format_exc()) logger.warning(f"Error fetching entities for pagination: {exc}") @@ -152,7 +153,10 @@ class SapErpClient: List all the columns on the SAP ERP instance """ try: - params_data = {"$filter": f"tabname eq '{table_name}'"} + logger.debug(f"Fetching columns for table {table_name}") + params_data = { + "$filter": f"tabname eq '{table_name}' and fieldname ne '.INCLUDE'" + } table_columns = self.paginate( api_url="/ECC/DDIC/ZZ_I_DDIC_COL_CDS/", params_data=params_data, diff --git a/ingestion/src/metadata/ingestion/source/database/saperp/constants.py b/ingestion/src/metadata/ingestion/source/database/saperp/constants.py index d2052c653ec..7cbcf4524fa 100644 --- a/ingestion/src/metadata/ingestion/source/database/saperp/constants.py +++ b/ingestion/src/metadata/ingestion/source/database/saperp/constants.py @@ -23,3 +23,5 @@ TABLE_TYPE_MAP = { "VIEW": TableType.View, "APPEND": TableType.View, } + +PARAMS_DATA = {"$top": "1", "$format": "json", "$inlinecount": "allpages"} diff --git a/ingestion/src/metadata/ingestion/source/database/saperp/metadata.py b/ingestion/src/metadata/ingestion/source/database/saperp/metadata.py index 8f8471895c7..2750b35129d 100644 --- a/ingestion/src/metadata/ingestion/source/database/saperp/metadata.py +++ b/ingestion/src/metadata/ingestion/source/database/saperp/metadata.py @@ -126,32 +126,24 @@ class SaperpSource(CommonDbSourceService): f"Unable to process table information for table: {str(table_name)} - {err}" ) - def _check_col_length( # pylint: disable=arguments-renamed - self, datatype: str + def _check_col_length( # pylint: disable=arguments-differ + self, datatype: str, col_length: Optional[str] ) -> Optional[int]: """ return the column length for the dataLength attribute """ - if datatype and datatype.upper() in {"CHAR", "VARCHAR", "BINARY", "VARBINARY"}: - return 1 - return None - - def _merge_col_descriptions(self, column: SapErpColumn) -> Optional[str]: - """ - Method to merge the column descriptions from different fields - """ - description = None try: - if column.scrtext_l: - description = f"**{column.scrtext_l}**" - if column.i_ddtext and column.scrtext_l != column.i_ddtext: - description = f"{description}\n{column.i_ddtext}" + if datatype and datatype.upper() in { + "CHAR", + "VARCHAR", + "BINARY", + "VARBINARY", + }: + return int(col_length) if col_length else None except Exception as exc: logger.debug(traceback.format_exc()) - logger.warning( - f"Unable to get column descriptions for {column.fieldname}: {exc}" - ) - return description + logger.warning(f"Failed to fetch column length: {exc}") + return None def _get_table_constraints( self, columns: Optional[List[Column]] @@ -197,6 +189,16 @@ class SaperpSource(CommonDbSourceService): return Constraint.PRIMARY_KEY return Constraint.NOT_NULL if column.notnull == "X" else Constraint.NULL + def _get_display_datatype( # pylint: disable=arguments-differ + self, column_type: str, col_data_length: Optional[int] + ) -> str: + """ + Method to get the display datatype + """ + if col_data_length: + return f"{column_type}({str(col_data_length)})" + return column_type + def get_columns_and_constraints( # pylint: disable=arguments-differ self, table_name: str ) -> ColumnsAndConstraints: @@ -209,7 +211,9 @@ class SaperpSource(CommonDbSourceService): for sap_column in sap_columns or []: try: column_type = ColumnTypeParser.get_column_type(sap_column.datatype) - data_type_display = column_type + col_data_length = self._check_col_length( + datatype=column_type, col_length=sap_column.leng + ) column_name = ( f"{sap_column.fieldname}({sap_column.precfield})" if sap_column.precfield @@ -221,6 +225,10 @@ class SaperpSource(CommonDbSourceService): logger.warning( f"Unknown type {repr(sap_column.datatype)}: {sap_column.fieldname}" ) + data_type_display = self._get_display_datatype( + column_type, col_data_length + ) + col_data_length = 1 if col_data_length is None else col_data_length om_column = Column( name=ColumnName( root=column_name @@ -229,15 +237,17 @@ class SaperpSource(CommonDbSourceService): if column_name else " " ), - displayName=sap_column.fieldname, - description=self._merge_col_descriptions(column=sap_column), + displayName=sap_column.scrtext_l + if sap_column.scrtext_l + else sap_column.fieldname, + description=sap_column.i_ddtext, dataType=column_type, dataTypeDisplay=data_type_display, ordinalPosition=int(sap_column.POS), constraint=self._get_column_constraint( column=sap_column, pk_columns=table_constraints_model.pk_columns ), - dataLength=self._check_col_length(datatype=column_type), + dataLength=col_data_length, ) if column_type == DataType.ARRAY.value: om_column.arrayDataType = DataType.UNKNOWN @@ -295,3 +305,6 @@ class SaperpSource(CommonDbSourceService): name=table.tabname, error=error, stackTrace=traceback.format_exc() ) ) + + def close(self): + self.metadata.close() diff --git a/ingestion/src/metadata/ingestion/source/database/saperp/models.py b/ingestion/src/metadata/ingestion/source/database/saperp/models.py index 1e8ec27cb06..3ca0a23f251 100644 --- a/ingestion/src/metadata/ingestion/source/database/saperp/models.py +++ b/ingestion/src/metadata/ingestion/source/database/saperp/models.py @@ -44,6 +44,7 @@ class SapErpColumn(BaseModel): scrtext_l: Optional[str] = None i_ddtext: Optional[str] = None dd_text: Optional[str] = None + leng: Optional[str] = None class SapErpTableList(BaseModel): diff --git a/ingestion/tests/unit/topology/database/test_saperp.py b/ingestion/tests/unit/topology/database/test_saperp.py index 1c919b4ea31..69df3d648df 100644 --- a/ingestion/tests/unit/topology/database/test_saperp.py +++ b/ingestion/tests/unit/topology/database/test_saperp.py @@ -111,16 +111,14 @@ EXPECTED_TABLES_AND_COLUMNS = [ columns=[ Column( name=ColumnName(root="BUKRS"), - displayName="BUKRS", + displayName="Pstng period variant", dataType="CHAR", arrayDataType=None, - dataLength=1, + dataLength=4, precision=None, scale=None, - dataTypeDisplay="CHAR", - description=Markdown( - root="**Pstng period variant**\nPosting Period Variant" - ), + dataTypeDisplay="CHAR(4)", + description=Markdown(root="Posting Period Variant"), fullyQualifiedName=None, tags=None, constraint=None, @@ -132,16 +130,14 @@ EXPECTED_TABLES_AND_COLUMNS = [ ), Column( name=ColumnName(root="FIELD"), - displayName="FIELD", + displayName="GL Field Name", dataType="CHAR", arrayDataType=None, - dataLength=1, + dataLength=30, precision=None, scale=None, - dataTypeDisplay="CHAR", - description=Markdown( - root="**GL Field Name**\nGeneral Ledger Field Name" - ), + dataTypeDisplay="CHAR(30)", + description=Markdown(root="General Ledger Field Name"), fullyQualifiedName=None, tags=None, constraint=None, @@ -153,14 +149,14 @@ EXPECTED_TABLES_AND_COLUMNS = [ ), Column( name=ColumnName(root="MANDT"), - displayName="MANDT", + displayName="Client", dataType="INT", arrayDataType=None, - dataLength=None, + dataLength=1, precision=None, scale=None, dataTypeDisplay="INT", - description=Markdown(root="**Client**"), + description=Markdown(root="Client"), fullyQualifiedName=None, tags=None, constraint=None, @@ -172,14 +168,14 @@ EXPECTED_TABLES_AND_COLUMNS = [ ), Column( name=ColumnName(root="RRCTY"), - displayName="RRCTY", + displayName="Record Type", dataType="CHAR", arrayDataType=None, dataLength=1, precision=None, scale=None, - dataTypeDisplay="CHAR", - description=Markdown(root="**Record Type**"), + dataTypeDisplay="CHAR(1)", + description=Markdown(root="Record Type"), fullyQualifiedName=None, tags=None, constraint=None, @@ -225,16 +221,14 @@ EXPECTED_TABLES_AND_COLUMNS = [ columns=[ Column( name=ColumnName(root="BKONT"), - displayName="BKONT", + displayName="To Account Assmnt", dataType="CHAR", arrayDataType=None, - dataLength=1, + dataLength=30, precision=None, scale=None, - dataTypeDisplay="CHAR", - description=Markdown( - root="**To Account Assmnt**\nTo Account Assignment" - ), + dataTypeDisplay="CHAR(30)", + description=Markdown(root="To Account Assignment"), fullyQualifiedName=None, tags=None, constraint=None, @@ -246,14 +240,14 @@ EXPECTED_TABLES_AND_COLUMNS = [ ), Column( name=ColumnName(root="BRGRU"), - displayName="BRGRU", + displayName="Authorization Group", dataType="CHAR", arrayDataType=None, - dataLength=1, + dataLength=4, precision=None, scale=None, - dataTypeDisplay="CHAR", - description=Markdown(root="**Authorization Group**"), + dataTypeDisplay="CHAR(4)", + description=Markdown(root="Authorization Group"), fullyQualifiedName=None, tags=None, constraint="NOT_NULL", @@ -265,16 +259,14 @@ EXPECTED_TABLES_AND_COLUMNS = [ ), Column( name=ColumnName(root="BUKRS"), - displayName="BUKRS", + displayName="Pstng period variant", dataType="CHAR", arrayDataType=None, - dataLength=1, + dataLength=4, precision=None, scale=None, - dataTypeDisplay="CHAR", - description=Markdown( - root="**Pstng period variant**\nPosting Period Variant" - ), + dataTypeDisplay="CHAR(4)", + description=Markdown(root="Posting Period Variant"), fullyQualifiedName=None, tags=None, constraint=None, @@ -370,7 +362,6 @@ class SapErpUnitTest(TestCase): returned_tables.extend( [either.right for either in self.saperp.yield_table(table)] ) - print(returned_tables) for _, (expected, original) in enumerate( zip(EXPECTED_TABLES_AND_COLUMNS, returned_tables) ): diff --git a/openmetadata-spec/src/main/resources/json/schema/entity/services/connections/database/sapErpConnection.json b/openmetadata-spec/src/main/resources/json/schema/entity/services/connections/database/sapErpConnection.json index 63c33ec767a..4e2320ce473 100644 --- a/openmetadata-spec/src/main/resources/json/schema/entity/services/connections/database/sapErpConnection.json +++ b/openmetadata-spec/src/main/resources/json/schema/entity/services/connections/database/sapErpConnection.json @@ -49,6 +49,13 @@ "type": "integer", "default": 10 }, + "verifySSL": { + "$ref": "../../../../security/ssl/verifySSLConfig.json#/definitions/verifySSL", + "default": "no-ssl" + }, + "sslConfig": { + "$ref": "../../../../security/ssl/verifySSLConfig.json#/definitions/sslConfig" + }, "connectionOptions": { "title": "Connection Options", "$ref": "../connectionBasicType.json#/definitions/connectionOptions"