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

Disallowing kylinpy 2.7.0 as it contains invalid pypi metadata #48226

Merged
merged 1 commit into from
Mar 25, 2025

Conversation

amoghrajesh
Copy link
Contributor

kylinpy 2.7.0 has a bad metadata on pypi as reported here: https://github.com/apache/airflow/actions/runs/14044194114/job/39321485583

Diasllowing that version. I also wonder if we need to bump since this version was from May 2020.


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

Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Just wondering >2.7.0 is existing and how was it working before? Did somebody update the previously released 2.7.0 with bad meta data but it was working before?!?

@eladkal
Copy link
Contributor

eladkal commented Mar 25, 2025

See Kyligence/kylinpy#57 (comment)

I also wonder if we need to bump since this version was from May 2020.

Not sure if it matters. Latest version is also from 2020. It seems like there is not much to do with this SDK (also only 2 open issues there) so it might make sense that it's oldish release but probably just fine as is.

@amoghrajesh
Copy link
Contributor Author

Just wondering >2.7.0 is existing and how was it working before? Did somebody update the previously released 2.7.0 with bad meta data but it was working before?!?

Its actually quite coincidental, the version is a range, so it can be picked up based on other constraints, seems it is just coincidence that this version gets picked up this time!

@amoghrajesh amoghrajesh merged commit b80a0ff into apache:main Mar 25, 2025
150 checks passed
@potiuk
Copy link
Member

potiuk commented Mar 25, 2025

Well. This was not the reason in this case. The reason was really the new setuptools - 78.0.1 - started to complain about the invalid metadata. But even latest kylinpy 2.8.4 has this invalid metadata:

[aliases]
test = pytest

[metadata]
description-file = README.rst    # !!!! THIS IS WRONG. Should be description_file !!!!

[egg_info]
tag_build = 
tag_date = 0

The problem with setuptools is that this is a default build-requirement for many packages. No matter what version you have locally, if your project has pyproject.toml, when a package needs to be built, it is built in isolation_mode by default - which means that whatever frontend you use (pip, hatch, uv) - it will create a separate, NEW virtualenv to build such project (kylinpy in this case) - it will install there LATEST requirements that the project declares as build dependencies and attempt to build the project in that venv.

There are MANY projects like kylinpy that do not pin their build requirements, This means tha whenever new setuptools is released, unless you specifiy --no-build-isolation for specific package (there are ways to individually specify which packages should be build without isolation, they will have a separately installed venv with latest setuptools - and when the latest setuptools does not like bad metadata, such package cannot be built.

There are several ways to deal with it:

  1. --no-build-isolation + pinning setuptools in your main venv (then such build will use whatever version of setuptools you already have in your local, shared venv where you run pip install or uv pip install

  2. when you use pip you can set PIP_CONSTRAINTS env variable and point it to a constraint file where you specify which version of setuptools (and other dependencies) should be used. Note - that only work via env variable, when you pass constraints with --constraints , those constrints are NOT passed to the isolated, new virtualenv

  3. when you use uv you can pass --build-constraints flag and pass constratints this way to the isolated venv where package is built. But then that flag applies to the whole uv pip install or uv sync or whatever command - so those constraints apply to all the packages that uv installs that needs building (i.e. all packages that have no .whl dstribution that matches your machine / architecture and so on.

So in this case - when next time setuptools maintainers will attempt to get rid of old, deprecated and "wrong" metadata packges, we either have to workaround it in one of the ways above (but then our users will have to do the same when installing airflow from PyPI - which is not ideal. Or better - we need to work with those dependencies of ours that have that problem and either help to (F)ix them or (F)ork them (I did it with json-merge-patch where I vendored in the 70 lines of code) or (F)orego them - i.e. switch to another dependency that is modern.

@potiuk
Copy link
Member

potiuk commented Mar 25, 2025

Just to clarify - no need to revert it BTW, it will work now with setuptools 78.0.2 where they brought back handling of "invalid" metadata, but this PR would not change anything if 78.0.1 was still the latest version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants