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 drivers/libraries used by anyone who uses mysql as
Airflow Metadata DB.
The pymssql driver installed in this step (together with binary libraries)
is used by the `apache-airflow-providers-microsoft-mssql` privider, NOT by
Airflow's SQLAlchemy to access Airflow's metadata DB:
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
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
