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

Restrict pydantic 2.10.0 #44249

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gopidesupavan
Copy link
Member

Recent pydantic 2.10.0 release causing failures in CI.
It seems there is ticket already opened here. pydantic/pydantic#10910

Failing tests in CI https://github.com/apache/airflow/actions/runs/11954326679/job/33324739597#step:7:12365


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

@gopidesupavan gopidesupavan added full tests needed We need to run full set of tests for this PR to merge all versions If set, the CI build will be forced to use all versions of Python/K8S/DBs labels Nov 21, 2024
@sydney-runkle
Copy link

We should have a patch release out this afternoon with a fix for the issues you're encountering :)

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

Not sure if we need to merge this or not given a 2.10.1 will be out soon

@ashb
Copy link
Member

ashb commented Nov 21, 2024

@sydney-runkle Do you plan on yanking the 2.10.0 release? (I don't mind either way, just need to know what we do)

If 2.10.0 gets yanked then we don't need to keep this/can revert it, otherwise we probably should keep the exclusion rule

hatch_build.py Outdated Show resolved Hide resolved
hatch_build.py Outdated Show resolved Hide resolved
Co-authored-by: Jarek Potiuk <[email protected]>
@sydney-runkle
Copy link

We're not going to yank v2.10.0, will just release a v2.10.1 patch soon!

@sydney-runkle
Copy link

Perhaps you all could test against our main branch before we do our patch release to confirm that all is working as expected on your end?

@potiuk
Copy link
Member

potiuk commented Nov 21, 2024

Perhaps you all could test against our main branch before we do our patch release to confirm that all is working as expected on your end?

It would be easier if you have an rc or beta/alpha released in PyPI - because I am not sure if we can trigger the whole test suite against Github-installed version. Can it be done @sydney-runkle ?

@sydney-runkle
Copy link

sydney-runkle commented Nov 21, 2024

You can try with pydantic==git+https://github.com/pydantic/pydantic@main

@potiuk
Copy link
Member

potiuk commented Nov 21, 2024

You can try with pydantic==git+https://github.com/pydantic/pydantic@main

It might be that we expect version in our CI for that, so I am not sure if it's going to work . I can try, but I have a feeling it will fail in one of the steps.

@gopidesupavan
Copy link
Member Author

You can try with pydantic==git+https://github.com/pydantic/pydantic@main

It might be that we expect version in our CI for that, so I am not sure if it's going to work . I can try, but I have a feeling it will fail in one of the steps.

yeah i think it might fail at constraints ?

@potiuk
Copy link
Member

potiuk commented Nov 21, 2024

You can try with pydantic==git+https://github.com/pydantic/pydantic@main

It might be that we expect version in our CI for that, so I am not sure if it's going to work . I can try, but I have a feeling it will fail in one of the steps.

Trying it here @sydney-runkle #44260

@potiuk
Copy link
Member

potiuk commented Nov 21, 2024

You can try with pydantic==git+https://github.com/pydantic/pydantic@main

It might be that we expect version in our CI for that, so I am not sure if it's going to work . I can try, but I have a feeling it will fail in one of the steps.

Trying it here @sydney-runkle #44260

Yeah... some tests are already failing here as I suspected. But those are auxiliary ones, maybe the main part of the tests that actually failed before will be ok:

  Preparing metadata (pyproject.toml): finished with status 'done'
ERROR: Requested apache-airflow==3.0.0.dev0 from file:///home/runner/work/airflow/airflow has invalid metadata: Expected end or semicolon (after name and no valid version specifier)
    pydantic==git+https://github.com/pydantic/pydantic@main

@Viicos
Copy link

Viicos commented Nov 21, 2024

We're going to release 2.10.1 any minute now.

We got a report today of an issue happening on Python <3.10. The TaskInstancePydantic class is subclassing LoggingMixin. This class has an annotation for _log using the new 3.10+ union syntax. While Pydantic has always evaluated all type annotations of models including their bases, we were not using the correct globals and locals to so. Meaning with the following example:

a.py

from logging import Logger

class Base:
    _log: 'Logger | None'

b.py

from a import Base

from pydantic import BaseModel

class Model(BaseModel, Base):
    pass

Evalutating the annotation for _log would have resulted in a NameError, because Logger is imported in a.py, and we only used the globals of the b.py module until now. NameError's are ignored to allow for types to be defined later on. However, on 2.10, this now raises a TypeError because 'Logger | None' successfully evaluates, but the union syntax is not supported on Python 3.9.

What's probably best is to make sure you're using the old syntax for every class that is going to be used in the context of a Pydantic model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
all versions If set, the CI build will be forced to use all versions of Python/K8S/DBs full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants