feat: Make SQLConnector _connect function public

[visch]

Feature scope

Taps (catalog, state, stream maps, tests, etc.)

Description

The function here https://github.com/meltano/sdk/blob/0285704e71dd9bce573c8c518b8ce3287943ce94/singer_sdk/connectors/sql.py#L82-L85

I'd like to change to the name connect as we use this in different taps.

See

• https://github.com/MeltanoLabs/tap-postgres/blob/5723435f8646f799edff51099fd474b97afb19df/tap_postgres/client.py#L305
• https://github.com/MeltanoLabs/tap-mysql/blob/c84d384ac8cddb2defbf1889128ba1bbdadf3549/tap_mysql/client.py#L68

[edgarrmondragon]

Thanks @visch!

I think the "private" _connect() method is only an issue if you use it outside of the SQLConnector implementation. For example, here: https://github.com/MeltanoLabs/tap-mysql/blob/c84d384ac8cddb2defbf1889128ba1bbdadf3549/tap_mysql/client.py#L427.

It hints that something should be implemented in MySQLConnector connector.

Note this was introduced in https://github.com/meltano/sdk/pull/1394 to ensure there's a single entry point for handling the connection object: https://github.com/meltano/sdk/issues/1393.

[visch]

I'm probably missing something @edgarrmondragon

I think the "private" _connect() method is only an issue if you use it outside of the SQLConnector implementation. https://github.com/meltano/sdk/blob/0285704e71dd9bce573c8c518b8ce3287943ce94/singer_sdk/streams/sql.py#L209-L217

The SDK does it too.

What triggered this for me is the linter told me I should so I added the code below. It just seemed weird to have to lint for the "standard" use case https://github.com/MeltanoLabs/tap-mysql/blob/c84d384ac8cddb2defbf1889128ba1bbdadf3549/tap_mysql/client.py#L427

We can just close and I can ignore it 🤷 , it's me being too pedantic

[edgarrmondragon]

No worries. We might end up doing this anyway if https://github.com/meltano/sdk/pull/1649 is shipped, or at least its Connector interface definition.