convert_urns_to_lowercase: inconsistency of usage in SQLAlchemySource.get_db & get_identifier methods

[sleeperdeep]

Describe the bug
Link to slack thread.

To Reproduce
Using

datahub/metadata-ingestion/src/datahub/ingestion/source/sql/sql_common.py

Lines 392 to 400 in 709c596

	def get_db_name(self, inspector: Inspector) -> str:
	engine = inspector.engine
	if engine and hasattr(engine, "url") and hasattr(engine.url, "database"):
	if engine.url.database is None:
	return ""
	return str(engine.url.database).strip('"').lower()
	else:
	raise Exception("Unable to get database name from Sqlalchemy inspector")

can lead to unexpected behaviour of urn creating. Despite of convert_urns_to_lowercase = True or False - database name will be in lowercase anyway.

Additional context
Investigating of relations between configs & modules leaded to next additional questions:
1.

datahub/metadata-ingestion/src/datahub/ingestion/source/sql/mssql/source.py

Line 64 in 709c596

class SQLServerConfig(BasicSQLAlchemyConfig):

has attribute

datahub/metadata-ingestion/src/datahub/ingestion/source/sql/mssql/source.py

Lines 98 to 101 in 709c596

	convert_urns_to_lowercase: bool = Field(
	default=False,
	description="Enable to convert the SQL Server assets urns to lowercase",
	)

but at the same moment LowerCaseDatasetUrnConfigMixin also have this attribute

datahub/metadata-ingestion/src/datahub/configuration/source_common.py

Lines 57 to 61 in 709c596

	class LowerCaseDatasetUrnConfigMixin(ConfigModel):
	convert_urns_to_lowercase: bool = Field(
	default=False,
	description="Whether to convert dataset urns to lowercase.",
	)

So, is this mistake or those attributes should perform different actions? If yes, why they have same name? If no, can we get rid off it from child class?

When I`m trying to find explicit using of this attributes in code, I see only next things:

a)

datahub/metadata-ingestion/src/datahub/ingestion/source/sql/mssql/source.py

Line 663 in 045c76a

if self.config.convert_urns_to_lowercase

b)

datahub/metadata-ingestion/src/datahub/ingestion/source/snowflake/snowflake_utils.py

Line 177 in 045c76a

if self.config.convert_urns_to_lowercase:

c)

datahub/metadata-ingestion/src/datahub/ingestion/api/source.py

Lines 208 to 229 in 045c76a

	auto_lowercase_dataset_urns: Optional[MetadataWorkUnitProcessor] = None
	if (
	self.ctx.pipeline_config
	and self.ctx.pipeline_config.source
	and self.ctx.pipeline_config.source.config
	and (
	(
	hasattr(
	self.ctx.pipeline_config.source.config,
	"convert_urns_to_lowercase",
	)
	and self.ctx.pipeline_config.source.config.convert_urns_to_lowercase
	)
	or (
	hasattr(self.ctx.pipeline_config.source.config, "get")
	and self.ctx.pipeline_config.source.config.get(
	"convert_urns_to_lowercase"
	)
	)
	)
	):
	auto_lowercase_dataset_urns = auto_lowercase_urns

I did not check it yet, but does it mean, that we can rid off all explicit using of convert_urns_to_lowercase attribute, because this parent class have method:

datahub/metadata-ingestion/src/datahub/ingestion/api/source_helpers.py

Lines 183 to 197 in 045c76a

	def auto_lowercase_urns(
	stream: Iterable[MetadataWorkUnit],
	) -> Iterable[MetadataWorkUnit]:
	"""Lowercase all dataset urns"""
	for wu in stream:
	try:
	old_urn = wu.get_urn()
	lowercase_dataset_urns(wu.metadata)
	wu.id = wu.id.replace(old_urn, wu.get_urn())
	yield wu
	except Exception as e:
	logger.warning(f"Failed to lowercase urns for {wu}: {e}", exc_info=True)
	yield wu

Or, if take a look at this from opposite point of view, I see only two explicit usage of convert_urns_to_lowercase attribute: in SQLServerSource and SnowflakeCommonMixin. Does it mean, that in all others sources this mechanizm (converting urns to lowercase) does not work?

@hsheth2 Can you, please, look at it one more time and after it I will create PR according to Slack thread. Thanks.

[github-actions]

This issue is stale because it has been open for 30 days with no activity. If you believe this is still an issue on the latest DataHub release please leave a comment with the version that you tested it with. If this is a question/discussion please head to https://slack.datahubproject.io. For feature requests please use https://feature-requests.datahubproject.io

[github-actions]

This issue was closed because it has been inactive for 30 days since being marked as stale.