testcontainers / testcontainers-python

Testcontainers is a Python library that providing a friendly API to run Docker container. It is designed to create runtime environment to use during your automatic tests.
https://testcontainers-python.readthedocs.io/en/latest/
Apache License 2.0
1.59k stars 290 forks source link

Use SqlServerContainer with pyodbc driver #244

Open gplssm opened 2 years ago

gplssm commented 2 years ago

In https://github.com/testcontainers/testcontainers-python/commit/fc5f2df4ba8157d21ecbf3a9a90cd45a61049f90#diff-313a71bf8d0b95ccd84a98f198294e10a7c7086c527efb513d9c6619a12086ee the default driver of SqlServerContainer was changed to pymssql. Currently, I use the pyodbc driver to communicate with MSSQL databases. This is not possible anymore since then. I've tried

test_container = SqlServerContainer(
        image="mcr.microsoft.com/mssql/server:2017-latest",
        dialect="mssql+pyodbc",
    )

which leads to a timeout testcontainers.core.exceptions.TimeoutException: Wait time (120s) exceeded for _connect(args: (), kwargs {}). Exception: (pyodbc.InterfaceError) ('IM002', '[IM002] [unixODBC][Driver Manager]Data source name not found and no default driver specified (0) (SQLDriverConnect)')

Alternatively, I've tried to specify the driver

test_container = SqlServerContainer(
        image="mcr.microsoft.com/mssql/server:2017-latest",
        dialect="mssql+pyodbc",
        driver = "ODBC Driver 17 for SQL Server"
    )

but this keyword isn't allowed anymore. > raise create_unexpected_kwargs_error('run', kwargs) E TypeError: run() got an unexpected keyword argument 'driver'

Is it still the goal of testcontainers to support using the pyodbc driver? If so, I might be motivated to support a little bit making this possible again in 3.6.x versions.

tillahoffmann commented 2 years ago

Based on https://github.com/testcontainers/testcontainers-python/commit/fc5f2df4ba8157d21ecbf3a9a90cd45a61049f90#diff-313a71bf8d0b95ccd84a98f198294e10a7c7086c527efb513d9c6619a12086eeL45, can you try adding with_env("SQLSERVER_DRIVER", "ODBC Driver 17 for SQL Server") and see whether that does the trick?

gplssm commented 2 years ago

Sorry for the long pause...

Yeah, that helps if we use the following as get_connection_url(self)

def get_connection_url(self):
standard_url = super()._create_connection_url(dialect="mssql+pyodbc",
                                              username=self.SQLSERVER_USER,
                                              password=self.SQLSERVER_PASSWORD,
                                              db_name=self.SQLSERVER_DBNAME,
                                              port=self.port_to_expose)
if "SQLSERVER_DRIVER" in self.env:
    return standard_url + "?driver=" + self.env["SQLSERVER_DRIVER"]
else:
    return standard_url

Would you be interested in a PR?

tillahoffmann commented 1 year ago

Maybe we can add a note to the documentation? I'm keen to drop pyodbc support because it requires the additional installation of drivers.

mortenbpost commented 1 year ago

Why would you want to get rid of pyodbc support? It's the only way to use an official microsoft driver and pyodbc is actively contributed to by Microsoft. This should be the preferred way. The support to use the driver was already there in the past. We are actually maintaining a fork just to support this. A PR to readd the functionality would be nice.

tillahoffmann commented 1 year ago

How about we add a driver argument as in #156?

gplssm commented 1 year ago

How about we add a driver argument as in #156?

I like the idea! And I think we can do it very similar. One difference between pymssql and pyodbc then will be that we have to append an "?driver=ODBC+Driver+17+for+SQL+Server" to the resulting URL for pyodbc.

I'll see if I can prepare a PR so that can discuss details along the code changes.