m32 / sqlalchemy-tds Goto Github PK

Hi,

Adding support for NTLM authentication would be really useful for my project.
I added a suggested pull request: #2

Let me know what you think!

Currently the host param seems to be required as part of the DB URI when creating a SQLAlchemy engine with sqlalchemy.create_engine:

# create connection pool
pool = sqlalchemy.create_engine(
    "mssql+pytds://<HOST>",
    creator=getconn,
)

However, some libraries that leverage SQLAlchemy's creator argument do not need a host param in order to successfully make a connection such as the Cloud SQL Python Connector. It seems that when the creator argument is used to establish the connection it does not even look at the host field, allowing you to put any dummy value for <HOST> and connections will still be established. This makes me believe that the param should be made optional.

Currently for MySQL and Postgres, connection with the Cloud SQL Python Connector are made as follows without the need for host:

from google.cloud.sql.connector import Connector
import sqlalchemy

# initialize Connector object
connector = Connector()

# function to return the database connection
def getconn() -> pymysql.connections.Connection:
    conn: pymysql.connections.Connection = connector.connect(
        "project:region:instance",
        "pymysql",
        user="my-user",
        password="my-password",
        db="my-db-name"
    )
    return conn

# create connection pool
pool = sqlalchemy.create_engine(
    "mysql+pymysql://",
    creator=getconn,
)

Ideal behavior for pytds would be as follows:

# create connection pool
pool = sqlalchemy.create_engine(
    "mssql+pytds://",
    creator=getconn,
)

If you try the above currently you get the following error:

File "/layers/google.python.pip/pip/lib/python3.10/site-packages/sqlalchemy_pytds/connector.py", line 99, in create_connect_args
  connect_args["dsn"] = connect_args["host"]
    KeyError: 'host'

Because of this we are currently telling users to pass in a dummy value as the host field to satisfy it being required:

# create connection pool
pool = sqlalchemy.create_engine(
    "mssql+pytds://dummy-value",
    creator=getconn,
)

is there any issue you see if we make it so that having a host field is not required?

Here is an example to the problem on stackoverflow.
https://stackoverflow.com/questions/71428691/insert-into-cloud-sql-sql-server-column-names-with-spaces-in-them

This code seems to work effectively to build a SQLAlchemy dialect for pytds. However, you can't pip install it. It looks like the project is registered at https://pypi.python.org/pypi/sqlalchemy-pytds but hasn't uploaded any files.