Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow to use dialect mssql+pyodbc #298

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Allow to use dialect mssql+pyodbc #298

wants to merge 7 commits into from

Conversation

gplssm
Copy link

@gplssm gplssm commented Jan 23, 2023 •
edited

Closes #244

@gplssm gplssm marked this pull request as ready for review January 23, 2023 16:11
tillahoffmann
tillahoffmann reviewed Jan 31, 2023
View reviewed changes
mssql/testcontainers/mssql/__init__.py Outdated
Comment on lines 68 to 69
if len(drivers) > 0:
driver_str = drivers[0].replace(" ", "+")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hunch is that this won't work because sorted is an iterable that can't be indexed. We likely also need to parse the version as an integer because, using string comparison, version 8 is greater than version 10. E.g.,

>>> max(['ODBC Driver 10 for SQL Server', 'ODBC Driver 8 for SQL Server'])
'ODBC Driver 8 for SQL Server'

tillahoffmann
tillahoffmann reviewed Feb 16, 2023
View reviewed changes
mssql/testcontainers/mssql/__init__.py

r = re.compile(r"ODBC Driver \d{1,2} for SQL Server")
# We sort drivers in reversed order to get the latest
drivers = sorted(list(filter(r.match, pyodbc.drivers())), reverse=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to convert to int first. We can probably just use max here rather than sorting and taking the first.

@tillahoffmann
Copy link
Collaborator

The test fails because Literal is not available in python 3.7 (cf python/typing#707). As 3.7 is close to end of life, let's omit the Literal typing for now, and we can re-introduce it once we drop support for python 3.7.

@tillahoffmann tillahoffmann linked an issue Feb 16, 2023 that may be closed by this pull request
Installation with poetry and mssqlserver extra broken #215
Open
@alexanderankin alexanderankin added the community-feat feature but its a community module so we wont bump tc core for it label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tillahoffmann tillahoffmann tillahoffmann left review comments

Assignees
No one assigned
Labels
community-feat feature but its a community module so we wont bump tc core for it 📦 package: mssql
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use SqlServerContainer with pyodbc driver Installation with poetry and mssqlserver extra broken
3 participants
@gplssm @tillahoffmann @alexanderankin