potiuk commented on code in PR #35868: URL: https://github.com/apache/airflow/pull/35868#discussion_r1406955218
########## Dockerfile: ########## @@ -342,87 +342,6 @@ if [[ ${INSTALL_MYSQL_CLIENT:="true"} == "true" ]]; then fi EOF -# The content below is automatically copied from scripts/docker/install_mssql.sh Review Comment: As explained on Slack - we likely do not want to remove this script. This script is to install MSSQL client libraries and pymssql client. Those are not necessarily the same as used by anyone who uses mysql as Airflow Metadata DB. The pymssql driver installed in this step (together with binary libraries) is used for sure by mssql provider https://github.com/apache/airflow/blob/main/airflow/providers/microsoft/mssql/hooks/mssql.py The driver that we recommend to use MSSQL as Metadata DB is a different one (it's `pyodbc` one) SQL Alchemy might use differnt drivers to communicate with MSSQL https://docs.sqlalchemy.org/en/20/dialects/mssql.html this is one ofht We are testing in CI using `mssql+pyodbc` driver not `mssql+pymssql`. I can't remember the details, but I believe there were issues with using pymssql driver. We might want to take a look at the history of adding MSSQL as core db. See: https://github.com/apache/airflow/blob/main/scripts/ci/docker-compose/backend-mssql.yml So the `pymssql` driver is part of the reference image of ours mostly because we want to be able to support `apache-airflow-microsoft-mssql` provider out-of-the-box as we recognise MSSQL as a database our users might often interact with. The goal of this PR is to remove any `MSSQL as Core Airflow Metadatadb backend` - not to remove pymssql library from the image. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org