fix(ingestion/kafka-connect): update retrieval of database name in Debezium SQL Server #8608
Conversation
github-actions
bot
added
the
ingestion
| @@ -686,7 +686,7 @@ def get_parser( | |||
| parser = self.DebeziumParser( | |||
| source_platform="mssql", | |||
| server_name=self.get_server_name(connector_manifest), | |||
| database_name=connector_manifest.config.get("database.dbname"), | |||
| database_name=connector_manifest.config.get("database.names") or connector_manifest.config.get("database.dbname"), | |||
There was a problem hiding this comment.
I believe this will work only if database.names has single database name, whereas database.names supports specifying multiple comma separated database names. Is that correct understanding ?
There was a problem hiding this comment.
Now that you mention it, I think that's right. In that specific case, you wouldn't be able to discern to which database the table belongs to with the information available in the manifest. If you picked only one of them, it could lead to erroneous lineage relationships.
Since the property database.dbname isn't available anymore for the SQL Server connector, could we support the single DB use case? If more than one DBs are detected, maybe a warning could be shown and the parameter database_name could be left empty.
There was a problem hiding this comment.
Yes, can do that. One small change suggested - when more than one DBs are detected, raise an exception, it'll get caught in the outer try..except block where we initialize DebeziumSourceConnectorLineages per connector. This will automatically report a warning . If we leave database_name empty, it'll generate incorrect table urns, without database name.
That being said, what do you think, should be ideal behavior if multiple database names are specified in connector ? What are the resolved topics in case multiple database.names (db1,db2) are specified, and both of databases have same schema name(schema1) and same table name(table1) inside it. Are changes to both tables (db1.schema1.table1 and db2.schema1.table1) written to same kafka topic or different ones ?
There was a problem hiding this comment.
Mmmh... I'm not sure, I haven't encountered that use case yet. Reading the docs it seems they'd be written to the same topic. Maybe the connector expects the table to exist in boths DBs and follow the same naming schemas.
How should that be translated on the lineage? Should we split the DBs and create a new lineage for each combination with the tables?
For the moment, could we finish this fix and setup a new PR with the proper scope?
There was a problem hiding this comment.
that makes sense to me.
There was a problem hiding this comment.
Ahh, my bad, the try..except is not present in parsing block of DebeziumSourceConnector._extract_lineages but only present in ConfluentS3SinkConnector._extract_lineages
Could you please add this try..except in DebeziumSourceConnector and then I can approve the PR.
There was a problem hiding this comment.
Sure thing! Just pushed the changes.
There's also some fixes requested by the linters, like casting database_name to str when checking if there's more than one DB.
mayurinehate
added
the
community-contribution
mayurinehate
added
the
pending-submitter-response
Starkie
force-pushed
the
fix/ingestion/kafka_connect-sqlserver-debezium-property
branch
from
fd91a2c
to
5d09068
Compare
…bezium SQL Server
Starkie
force-pushed
the
fix/ingestion/kafka_connect-sqlserver-debezium-property
branch
from
5d09068
to
9f80467
Compare
mayurinehate
removed
the
pending-submitter-response
asikowitz
added
the
merge-pending-ci
|
unrelated failure so merging this in. |
In the version 2.0 of the Debezium SQL Server connector the name of the property
database.dbnamechanged todatabase.names. I've checked the other supported connectors and it seems this is the only one affected.In this PR I've changed the default property to retrieve from the connector's manifest to
database.namesand left thedatabase.dbnameas a fallback.Checklist