potiuk commented on code in PR #35868:
URL: https://github.com/apache/airflow/pull/35868#discussion_r1406958011
##########
Dockerfile:
##########
@@ -1259,29 +1176,19 @@ RUN bash /scripts/docker/install_os_dependencies.sh dev
ARG INSTALL_MYSQL_CLIENT="true"
ARG INSTALL_MYSQL_CLIENT_TYPE="mysql"
-ARG INSTALL_MSSQL_CLIENT="true"
ARG INSTALL_POSTGRES_CLIENT="true"
ARG AIRFLOW_PIP_VERSION
ENV INSTALL_MYSQL_CLIENT=${INSTALL_MYSQL_CLIENT} \
INSTALL_MYSQL_CLIENT_TYPE=${INSTALL_MYSQL_CLIENT_TYPE} \
- INSTALL_MSSQL_CLIENT=${INSTALL_MSSQL_CLIENT} \
INSTALL_POSTGRES_CLIENT=${INSTALL_POSTGRES_CLIENT}
-# Only copy mysql/mssql installation scripts for now - so that changing the
other
+# Only copy mysql installation scripts for now - so that changing the other
# scripts which are needed much later will not invalidate the docker layer here
-COPY --from=scripts install_mysql.sh install_mssql.sh install_postgres.sh
/scripts/docker/
-
-# THE 3 LINES ARE ONLY NEEDED IN ORDER TO MAKE PYMSSQL BUILD WORK WITH LATEST
CYTHON
Review Comment:
As explained above - I think we do not want to remove it in this PR. We
might want to see if we still need to use the Cython workaround to bulld
pymssql package on ARM images.
Cython compatibility seems to be fixed in
https://github.com/pymssql/pymssql/blob/master/ChangeLog.rst#version-228---2023-07-30----mikhail-terekhov
> Compatibility with Cython. Thanks to matusvalo (Matus Valo) (fix #826).
So likely yes - we can remove the workaround: but IMHO it shoudl be a
separate PR reverting changes imple
mented in https://github.com/apache/airflow/pull/32748 - and we shoudl not
remove `pymssql`
--
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
