Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jul 21, 2023

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:

Since we have to change Dockerfile around installing pip, also version of pip has been upgraded to latest - 23.2


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@potiuk potiuk requested a review from ashb as a code owner July 21, 2023 15:10
@boring-cyborg boring-cyborg bot added area:dev-tools area:production-image Production image improvements and fixes kind:documentation labels Jul 21, 2023
@potiuk potiuk force-pushed the fix-cython-compatibility-for-arm branch 4 times, most recently from d1c28f9 to f7eddae Compare July 21, 2023 16:16
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:

* cython/cython#5541
* pymssql/pymssql#827
* https://discuss.python.org/t/no-way-to-pin-build-dependencies/29833

Since we have to change Dockerfile around installing `pip`, also
version of `pip` has been upgraded to latest - 23.2
@potiuk potiuk force-pushed the fix-cython-compatibility-for-arm branch from f7eddae to 8d291f8 Compare July 21, 2023 16:18
@potiuk potiuk requested a review from jedcunningham as a code owner July 21, 2023 16:18
@potiuk
Copy link
Member Author

potiuk commented Jul 21, 2023

OK. Seems that this one is fixing ARM image building :)

@potiuk potiuk merged commit 8e054bb into apache:main Jul 21, 2023
@potiuk potiuk deleted the fix-cython-compatibility-for-arm branch July 21, 2023 17:27
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 28, 2023
Recent Cython release caused pymssql package failures when they
were installed on ARM platform. This had been workarounded in
the apache#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.
potiuk added a commit that referenced this pull request Nov 28, 2023
…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.
ephraimbuddy pushed a commit that referenced this pull request Dec 5, 2023
…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.

(cherry picked from commit 4f060a4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants