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.

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

Replace pymssql with pyodbc #3808

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattiasthalen
Copy link

Replace pymssql in mssql in connectors with pyodbc.

This opens the ability to use other kind of authorisations, e.g., service principal. It will also be needed in the future in order to integrate with Microsoft Fabric.

@CLAassistant
Copy link

CLAassistant commented Feb 8, 2025

CLA assistant check
All committers have signed the CLA.

@mattiasthalen
Copy link
Author

Failing ci due to pyodbc not being in the requirements. I've tried finding where to put it, but no luck. Any hints?

@erindru
Copy link
Collaborator

erindru commented Feb 9, 2025

Failing ci due to pyodbc not being in the requirements.

Check setup.py, extras_require for mssql

However - I don't think we want to replace pymssql, because that will break SQLMesh for every project that is currently using MSSQL the second they upgrade SQLMesh (by introducing a requirement to have ODBC set up and ODBC drivers installed)

Have you considered implementing this as an option instead? Something like adding a driver parameter the config. It could default to driver=pymssql and retain the current behaviour, or could be set to driver=odbc to trigger the new behaviour

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants