Skip to content

Commit

Permalink
Remove workaround for pymssql failing compilation with new Cython (#3…
Browse files Browse the repository at this point in the history
…5924)

Recent Cython release caused pymssql package failures when they
were installed on ARM platform. This had been workarounded in
the #32748, but since pymssql as of 2.1.8 supports new Cython, we
can remove the workaround and bump the minimum version of pymsssql.

This also makes it possible to remove the whole MSSQL client section
from the image if we decide to - because this section will only install
the odbc client that has been pre-installed to support MSSQL as
metadata DB for Airflow core.
  • Loading branch information
potiuk committed Nov 28, 2023
1 parent 2919abe commit 4f060a4
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 119 deletions.
42 changes: 0 additions & 42 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,6 @@ COPY <<"EOF" /install_mssql.sh
#!/usr/bin/env bash
set -euo pipefail

. "$( dirname "${BASH_SOURCE[0]}" )/common.sh"

: "${AIRFLOW_PIP_VERSION:?Should be set}"

: "${INSTALL_MSSQL_CLIENT:?Should be true or false}"
Expand Down Expand Up @@ -384,40 +382,6 @@ function install_mssql_client() {
rm -rf /var/lib/apt/lists/*
apt-get autoremove -yqq --purge
apt-get clean && rm -rf /var/lib/apt/lists/*

# Workaround an issue with installing pymssql on ARM architecture triggered by Cython 3.0.0 release as of
# 18 July 2023. The problem is that pip uses latest Cython to compile pymssql and since we are using
# setuptools, there is no easy way to fix version of Cython used to compile packages.
#
# This triggers a problem with newer `pip` versions that have build isolation enabled by default because
# There is no (easy) way to pin build dependencies for dependent packages. If a package does not have
# limit on build dependencies, it will use the latest version of them to build that particular package.
#
# The workaround to the problem suggest in the last thread by Pradyun Gedam - pip maintainer - is to
# use PIP_CONSTRAINT environment variable and constraint the version of Cython used while installing
# the package. Which is precisely what we are doing here.
#
# Note that it does not work if we pass ``--constraint`` option to pip because it will not be passed to
# the package being build in isolation. The fact that the PIP_CONSTRAINT env variable works in the isolation
# is a bit of side-effect on how env variables work and that they are passed to subprocesses as pip
# launches a subprocess `pip` to build the package.
#
# This is a temporary solution until the issue is resolved in pymssql or Cython
# Issues/discussions that track it:
#
# * https://github.com/cython/cython/issues/5541
# * https://github.com/pymssql/pymssql/pull/827
# * https://discuss.python.org/t/no-way-to-pin-build-dependencies/29833
#
# TODO: Remove this workaround when the issue is resolved.
# ALSO REMOVE THE TOP LINES ABOVE WITH common.sh IMPORT AS WELL AS COPYING common.sh ib
# Dockerfile AND Dockerfile.ci (look for capital PYMSSQL - there are several places to remove)
if [[ "${1}" == "dev" ]]; then
common::install_pip_version
echo "Cython==0.29.36" >> /tmp/mssql-constraints.txt
PIP_CONSTRAINT=/tmp/mssql-constraints.txt pip install pymssql
rm /tmp/mssql-constraints.txt
fi
}

install_mssql_client "${@}"
Expand Down Expand Up @@ -1272,12 +1236,6 @@ ENV INSTALL_MYSQL_CLIENT=${INSTALL_MYSQL_CLIENT} \
# 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
# AND SHOULD BE REMOVED WHEN WORKAROUND IN install_mssql.sh IS REMOVED
ARG AIRFLOW_PIP_VERSION=23.3.1
ENV AIRFLOW_PIP_VERSION=${AIRFLOW_PIP_VERSION}
COPY --from=scripts common.sh /scripts/docker/

RUN bash /scripts/docker/install_mysql.sh dev && \
bash /scripts/docker/install_mssql.sh dev && \
bash /scripts/docker/install_postgres.sh dev
Expand Down
36 changes: 0 additions & 36 deletions Dockerfile.ci
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,6 @@ COPY <<"EOF" /install_mssql.sh
#!/usr/bin/env bash
set -euo pipefail

. "$( dirname "${BASH_SOURCE[0]}" )/common.sh"

: "${AIRFLOW_PIP_VERSION:?Should be set}"

: "${INSTALL_MSSQL_CLIENT:?Should be true or false}"
Expand Down Expand Up @@ -344,40 +342,6 @@ function install_mssql_client() {
rm -rf /var/lib/apt/lists/*
apt-get autoremove -yqq --purge
apt-get clean && rm -rf /var/lib/apt/lists/*

# Workaround an issue with installing pymssql on ARM architecture triggered by Cython 3.0.0 release as of
# 18 July 2023. The problem is that pip uses latest Cython to compile pymssql and since we are using
# setuptools, there is no easy way to fix version of Cython used to compile packages.
#
# This triggers a problem with newer `pip` versions that have build isolation enabled by default because
# There is no (easy) way to pin build dependencies for dependent packages. If a package does not have
# limit on build dependencies, it will use the latest version of them to build that particular package.
#
# The workaround to the problem suggest in the last thread by Pradyun Gedam - pip maintainer - is to
# use PIP_CONSTRAINT environment variable and constraint the version of Cython used while installing
# the package. Which is precisely what we are doing here.
#
# Note that it does not work if we pass ``--constraint`` option to pip because it will not be passed to
# the package being build in isolation. The fact that the PIP_CONSTRAINT env variable works in the isolation
# is a bit of side-effect on how env variables work and that they are passed to subprocesses as pip
# launches a subprocess `pip` to build the package.
#
# This is a temporary solution until the issue is resolved in pymssql or Cython
# Issues/discussions that track it:
#
# * https://github.com/cython/cython/issues/5541
# * https://github.com/pymssql/pymssql/pull/827
# * https://discuss.python.org/t/no-way-to-pin-build-dependencies/29833
#
# TODO: Remove this workaround when the issue is resolved.
# ALSO REMOVE THE TOP LINES ABOVE WITH common.sh IMPORT AS WELL AS COPYING common.sh ib
# Dockerfile AND Dockerfile.ci (look for capital PYMSSQL - there are several places to remove)
if [[ "${1}" == "dev" ]]; then
common::install_pip_version
echo "Cython==0.29.36" >> /tmp/mssql-constraints.txt
PIP_CONSTRAINT=/tmp/mssql-constraints.txt pip install pymssql
rm /tmp/mssql-constraints.txt
fi
}

install_mssql_client "${@}"
Expand Down
2 changes: 1 addition & 1 deletion airflow/providers/microsoft/mssql/provider.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ versions:
dependencies:
- apache-airflow>=2.5.0
- apache-airflow-providers-common-sql>=1.3.1
- pymssql>=2.1.5
- pymssql>=2.1.8

integrations:
- integration-name: Microsoft SQL Server (MSSQL)
Expand Down
2 changes: 1 addition & 1 deletion generated/provider_dependencies.json
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@
"deps": [
"apache-airflow-providers-common-sql>=1.3.1",
"apache-airflow>=2.5.0",
"pymssql>=2.1.5"
"pymssql>=2.1.8"
],
"cross-providers-deps": [
"common.sql"
Expand Down
39 changes: 0 additions & 39 deletions scripts/docker/install_mssql.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@
# shellcheck shell=bash
set -euo pipefail

# REMOVE THOSE 3 LINES BELOW TOGETHER WITH PYMSSQL INSTALLATION BELOW AFTER PYMSSQL
# INSTALLATION WITH CYTHON 3.0.0 IS FIXED SEE BELOW FOR MORE DETAILS
# shellcheck source=scripts/docker/common.sh
. "$( dirname "${BASH_SOURCE[0]}" )/common.sh"

: "${AIRFLOW_PIP_VERSION:?Should be set}"

: "${INSTALL_MSSQL_CLIENT:?Should be true or false}"
Expand Down Expand Up @@ -58,40 +53,6 @@ function install_mssql_client() {
rm -rf /var/lib/apt/lists/*
apt-get autoremove -yqq --purge
apt-get clean && rm -rf /var/lib/apt/lists/*

# Workaround an issue with installing pymssql on ARM architecture triggered by Cython 3.0.0 release as of
# 18 July 2023. The problem is that pip uses latest Cython to compile pymssql and since we are using
# setuptools, there is no easy way to fix version of Cython used to compile packages.
#
# This triggers a problem with newer `pip` versions that have build isolation enabled by default because
# There is no (easy) way to pin build dependencies for dependent packages. If a package does not have
# limit on build dependencies, it will use the latest version of them to build that particular package.
#
# The workaround to the problem suggest in the last thread by Pradyun Gedam - pip maintainer - is to
# use PIP_CONSTRAINT environment variable and constraint the version of Cython used while installing
# the package. Which is precisely what we are doing here.
#
# Note that it does not work if we pass ``--constraint`` option to pip because it will not be passed to
# the package being build in isolation. The fact that the PIP_CONSTRAINT env variable works in the isolation
# is a bit of side-effect on how env variables work and that they are passed to subprocesses as pip
# launches a subprocess `pip` to build the package.
#
# This is a temporary solution until the issue is resolved in pymssql or Cython
# Issues/discussions that track it:
#
# * https://github.com/cython/cython/issues/5541
# * https://github.com/pymssql/pymssql/pull/827
# * https://discuss.python.org/t/no-way-to-pin-build-dependencies/29833
#
# TODO: Remove this workaround when the issue is resolved.
# ALSO REMOVE THE TOP LINES ABOVE WITH common.sh IMPORT AS WELL AS COPYING common.sh ib
# Dockerfile AND Dockerfile.ci (look for capital PYMSSQL - there are several places to remove)
if [[ "${1}" == "dev" ]]; then
common::install_pip_version
echo "Cython==0.29.36" >> /tmp/mssql-constraints.txt
PIP_CONSTRAINT=/tmp/mssql-constraints.txt pip install pymssql
rm /tmp/mssql-constraints.txt
fi
}

install_mssql_client "${@}"

0 comments on commit 4f060a4

Please sign in to comment.