Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zoneinfo.ZoneInfo is not available on < Python 3.9 #34804

Merged
merged 4 commits into from
Oct 6, 2023

Conversation

bolkedebruin
Copy link
Contributor

Tests were not functioning due to checks if classes could be imported from serializers.

@Taragolis @ferruzzi


^ 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.

Tests were not functioning due to checks if classes
could be imported from serializers.
@bolkedebruin
Copy link
Contributor Author

I need some help here, cause I am not sure how to fix this? Maybe @uranusjr @Taragolis ?

setup.py Outdated
"backports.zoneinfo>=0.2.1;python_version<'3.9'",
"backports.zoneinfo>=0.2.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have to limit backports.zoneinfo

The builds failed in python 3.10 and 3.11

#55 195.8   × Building wheel for backports.zoneinfo (pyproject.toml) did not run successfully.
  #55 195.8   │ exit code: 1
  #55 195.8   ╰─> [55 lines of output]
  #55 195.8       /tmp/pip-build-env-2iu_fpi3/overlay/lib/python3.10/site-packages/setuptools/config/setupcfg.py:293: _DeprecatedConfig: Deprecated config in `setup.cfg`
  #55 195.8       !!
  #55 195.8       
  #55 195.8               ********************************************************************************
  #55 195.8               The license_file parameter is deprecated, use license_files instead.
  #55 195.8       
  #55 195.8               By 2023-Oct-30, you need to update your project and remove deprecated calls
  #55 195.8               or your builds will no longer be supported.
  #55 195.8       
  #55 195.8               See https://setuptools.pypa.io/en/latest/userguide/declarative_config.html for details.
  #55 195.8               ********************************************************************************
  #55 195.8       
  #55 195.8       !!
  #55 195.8         parsed = self.parsers.get(option_name, lambda x: x)(value)
  #55 195.8       running bdist_wheel
  #55 195.8       running build
  #55 195.8       running build_py
  #55 195.8       creating build
  #55 195.8       creating build/lib.linux-x86_64-cpython-310
  #55 195.8       creating build/lib.linux-x86_64-cpython-310/backports
  #55 195.8       copying src/backports/__init__.py -> build/lib.linux-x86_64-cpython-310/backports
  #55 195.8       creating build/lib.linux-x86_64-cpython-310/backports/zoneinfo
  #55 195.8       copying src/backports/zoneinfo/_zoneinfo.py -> build/lib.linux-x86_64-cpython-310/backports/zoneinfo
  #55 195.8       copying src/backports/zoneinfo/_version.py -> build/lib.linux-x86_64-cpython-310/backports/zoneinfo
  #55 195.8       copying src/backports/zoneinfo/_tzpath.py -> build/lib.linux-x86_64-cpython-310/backports/zoneinfo
  #55 195.8       copying src/backports/zoneinfo/_common.py -> build/lib.linux-x86_64-cpython-310/backports/zoneinfo
  #55 195.8       copying src/backports/zoneinfo/__init__.py -> build/lib.linux-x86_64-cpython-310/backports/zoneinfo
  #55 195.8       running egg_info
  #55 195.8       writing src/backports.zoneinfo.egg-info/PKG-INFO
  #55 195.8       writing dependency_links to src/backports.zoneinfo.egg-info/dependency_links.txt
  #55 195.8       writing requirements to src/backports.zoneinfo.egg-info/requires.txt
  #55 195.8       writing top-level names to src/backports.zoneinfo.egg-info/top_level.txt
  #55 195.8       reading manifest file 'src/backports.zoneinfo.egg-info/SOURCES.txt'
  #55 195.8       reading manifest template 'MANIFEST.in'
  #55 195.8       warning: no files found matching '*.png' under directory 'docs'
  #55 195.8       warning: no files found matching '*.svg' under directory 'docs'
  #55 195.8       no previously-included directories found matching 'docs/_build'
  #55 195.8       no previously-included directories found matching 'docs/_output'
  #55 195.8       adding license file 'LICENSE'
  #55 195.8       adding license file 'licenses/LICENSE_APACHE'
  #55 195.8       writing manifest file 'src/backports.zoneinfo.egg-info/SOURCES.txt'
  #55 195.8       copying src/backports/zoneinfo/__init__.pyi -> build/lib.linux-x86_64-cpython-310/backports/zoneinfo
  #55 195.8       copying src/backports/zoneinfo/py.typed -> build/lib.linux-x86_64-cpython-310/backports/zoneinfo
  #55 195.8       running build_ext
  #55 195.8       building 'backports.zoneinfo._czoneinfo' extension
  #55 195.8       creating build/temp.linux-x86_64-cpython-310
  #55 195.8       creating build/temp.linux-x86_64-cpython-310/lib
  #55 195.8       gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/usr/local/include/python3.10 -c lib/zoneinfo_module.c -o build/temp.linux-x86_64-cpython-310/lib/zoneinfo_module.o -std=c99
  #55 195.8       lib/zoneinfo_module.c: In function ‘zoneinfo_fromutc’:
  #55 195.8       lib/zoneinfo_module.c:600:19: error: ‘_PyLong_One’ undeclared (first use in this function); did you mean ‘_PyLong_New’?
  #55 195.8         600 |             one = _PyLong_One;
  #55 195.8             |                   ^~~~~~~~~~~
  #55 195.8             |                   _PyLong_New
  #55 195.8       lib/zoneinfo_module.c:600:19: note: each undeclared identifier is reported only once for each function it appears in
  #55 195.8       error: command '/usr/bin/gcc' failed with exit code 1
  #55 195.8       [end of output]
  #55 195.8   
  #55 195.8   note: This error originates from a subprocess, and is likely not a problem with pip.
  #55 195.8   ERROR: Failed building wheel for backports.zoneinfo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/pganssle/zoneinfo#installation-and-depending-on-this-library

Support for backports.zoneinfo in Python 3.9+ is currently minimal, since it is expected that you would use the standard library zoneinfo module instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so we need to exclude it indeed if PY > 3.9 and also not make it part of the serializers then

Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
@vincbeck vincbeck merged commit 25cd12d into apache:main Oct 6, 2023
43 checks passed
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.2 milestone Oct 6, 2023
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Oct 6, 2023
ephraimbuddy pushed a commit that referenced this pull request Oct 7, 2023
@eric-saintetienne
Copy link

eric-saintetienne commented Nov 14, 2023

Actually, this is not fixed in python versions prior to 3.9 on arm64.

For example try to install tzlocal on apache/airflow:2.3.4-python3.7 and you'll see that pip is trying to build backport.zoneinfo from sources because there's no wheel available for arm64, the compilation of which fails because gcc isn't installed in the airflow image. Add insult to injury, it's not possible to install gcc because the image is locked to a non-root user.

tzlocal is pulled as dependency of another package that we need to install inside the image (something used by our code running in airflow dags)

There's no wheel for that package on arm64: pganssle/zoneinfo#100 (comment)

@potiuk
Copy link
Member

potiuk commented Nov 15, 2023

Actually, this is not fixed in python versions prior to 3.9 on arm64.

For example try to install tzlocal on apache/airflow:2.3.4-python3.7 and you'll see that pip is trying to build backport.zoneinfo from sources because there's no wheel available for arm64, the compilation of which fails because gcc isn't installed in the airflow image. Add insult to injury, it's not possible to install gcc because the image is locked to a non-root user.

tzlocal is pulled as dependency of another package that we need to install inside the image (something used by our code running in airflow dags)

There's no wheel for that package on arm64: pganssle/zoneinfo#100 (comment)

Sure. As you can see here it's fixed in Python 2.7.2 so image 2.3.4 does not contain the package installed. The ARM version of the image has an experimental status (precisely because we are not runnig all tests on it) - this might change when we have ARM runners in the future, but for now you need to handle it yourself.

Obviously you can build your own image and it's described how to do it - and even how to add packages the require compilation - straight in our docker image documentation (it includes the very example of adding packages that require complilation - installing build-essentials after changing the user to root and swtching to airflow before) https://airflow.apache.org/docs/docker-stack/build.html#example-when-you-add-packages-requiring-compilation - you can follow the docs and build your own image.

@eric-saintetienne
Copy link

eric-saintetienne commented Nov 15, 2023

Thanks, I didn't know the arm64 version of airflow was still experimental. Thanks for the advice, in the end I simply used the python 3.9 version of the airflow-2.3.4 arm image.

That said, I'm considering running airflow in production on arm64 servers, how far is it from being production ready? I've searched for a page with the roadmap or status of the arm build, but couldn't find one.

Edit: I've found this github issue: #15635 that deals with building an arm64 image but it's been closed as well as the corresponding epic.

@uranusjr
Copy link
Member

Eh, this is not the right place for the discussion. Airflow does not require zoneinfo at all; this simply improves behaviour when another package passes in a ZoneInfo object to Airflow. The fact you hit an error on installation time has nothing to do with Airflow, but that other package you depend on. You need to talk to maintainers of that package instead to resolve this; Airflow maintainers cannot possibly fix that for you.

@eric-saintetienne
Copy link

eric-saintetienne commented Nov 15, 2023

You're right, zoneinfo is not a direct dependency of airflow.

The problem is that airflow locks down the image and by doing so doesn't make it possible to install such dependency without having to resort to write your own Dockerfile.

@potiuk
Copy link
Member

potiuk commented Nov 15, 2023

That said, I'm considering running airflow in production on arm64 servers, how far is it from being production ready? I've searched for a page with the roadmap or status of the arm build, but couldn't find one.

It used for development mostly and it waits until we run a complete suite of tests on ARM to be able to remove the "experimental" tag - which is going to happen when we switch our CI infrastructure to K8S-run when we will be able to chess the runner freely.

The problem is that airflow locks down the image and by doing so doesn't make it possible to install such dependency without having to resort to write your own Dockerfile.

It's simply not true. Airflow does not "lock-down" anything. It sets user to non-root (as all production images should do). And you can still change the user to root for isntallation . This is not only possible, but explained in our documentation and examples as I pointed out. Just read it. If you read the docs, you can also find out that this is highly optimized image that deliberatly does not have build-essentials. Also if you read them further you will find that Airflow image is "reference" one and if you have any needs that go beyond it - you can also use airflow Dockerfile to build your own, customized version of it that fits your need. Ultimately there are no "guarantees" this image gives anyway and you are responsible to make sure you have what you need for your production need.

BTW. Airflow (and it's image) comes for fee and with no guarantees whatsoever so the experimental status is nothing more than indication we are not fully testing it in CI and tha things might change abruptly there as we might find a need to add/remove thigns in next version in order to fix some problems we are not aware of.

So this is not an issuse of Airflow at all anyway - regardless.

Still I am happy to point you to the docs so that you can do whatever you need for your case.

@potiuk
Copy link
Member

potiuk commented Nov 15, 2023

So basically to summarize it - it's yours problem to solve and you have all the tools and information needed to solve them :)

@eric-saintetienne
Copy link

Thanks for the context about arm build. What would be the best place to watch for updates about the arm64 status?

@potiuk
Copy link
Member

potiuk commented Nov 15, 2023

Nothing different than usual - devlist and user list - subscribe to it and all the important announcements are there (Also security CVE advisories, release announcements etc. You will find all details about it in "community" tab of our documentation.

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.

8 participants