-
Notifications
You must be signed in to change notification settings - Fork 40
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
linux/arm64 support, mssql17 -> 18 #1034
base: 2.0
Are you sure you want to change the base?
Conversation
There are issues on lapp with such changes:
I will try to figure out proper certificate for it later. |
In ODBC Driver for SQL Server 17, by default there were no certificate checked. They changed it and in 18 it's enabled, that's why there is such error on Lapp:
It should be possible to set these parameters in pyodbc and then it should work again, but it needs to be tested by someone, eg. @angelika233 before confirming:
|
Yeah, we should allow passing extra connection args in Sth like (this is probably buggy but you get the idea 😛) Also, these names are probably case-insensitive so this could be simplified to not bother with capitalization. @property
def conn_str(self) -> str:
"""Generate a connection string from params or config.
Note that the user and password are escaped with '{}' characters.
Returns:
str: The ODBC connection string.
"""
def params_dict_to_connection_string(params):
return ';'.join([f"{key}={value}" for key, value in params.items()]) + ';'
credentials = copy.deepcopy(self.credentials)
driver = credentials.pop("driver")
server = credentials.pop("server")
db_name = credentials.pop("db_name")
uid = credentials.pop("user", "")
pwd = credentials.pop("password", "")
conn_str = f"DRIVER={{{driver}}};SERVER={server};DATABASE={db_name};UID={uid};PWD={pwd};"
# Should probably also be handled with the extra_args logic.
if auth := credentials.pop("authentication", ""):
conn_str += "Authentication=" + auth + ";"
extra_args = params_dict_to_connection_string(credentials)
return conn_str + extra_args |
I tested it by changing the SQL source in the following way:
I tried different combinations of these params but it didn't help with the error. The error message stays the same |
@angelika233 |
Might be this:
Source: https://docs.openssl.org/3.0/man7/migration_guide/#tls-changes, through https://stackoverflow.com/a/77814036/8524524 Here's also a supposedly fully working example: https://techcommunity.microsoft.com/t5/sql-server-blog/odbc-driver-18-0-for-sql-server-released/ba-p/3169228 (comment by user fetzere) |
Summary
linux/arm64 support added
msodbcsql17 -> 18, as arm64 packages are available only with v18
Importance
multiplatform support is useful for people using different architecture than amd64
Checklist
This PR:
CONTRIBUTING.md
CHANGELOG.md