You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "uranusjr (via GitHub)" <gi...@apache.org> on 2023/07/05 08:43:57 UTC
[GitHub] [airflow] uranusjr opened a new pull request, #32366: Support Pydantic 2
uranusjr opened a new pull request, #32366:
URL: https://github.com/apache/airflow/pull/32366
Fix https://github.com/apache/airflow/issues/32311 eventually. For CI only at the moment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #32366: Support Pydantic 2
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #32366:
URL: https://github.com/apache/airflow/pull/32366#discussion_r1255247622
##########
Dockerfile:
##########
@@ -385,7 +385,8 @@ function install_airflow_dependencies_from_branch_tip() {
pip install --root-user-action ignore \
${ADDITIONAL_PIP_INSTALL_FLAGS} \
"https://github.com/${AIRFLOW_REPO}/archive/${AIRFLOW_BRANCH}.tar.gz#egg=apache-airflow[${AIRFLOW_EXTRAS}]" \
- --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}" || true
+ --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}" \
+ || pip install 'pydantic>=2' || true
Review Comment:
It would resolve it to 1.18 because (a dependency of) Moto pins it to `pydantic~=1.18` (I tried it locally before settling on this hack)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #32366: Support Pydantic 2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #32366:
URL: https://github.com/apache/airflow/pull/32366#discussion_r1254273652
##########
Dockerfile:
##########
@@ -385,7 +385,8 @@ function install_airflow_dependencies_from_branch_tip() {
pip install --root-user-action ignore \
${ADDITIONAL_PIP_INSTALL_FLAGS} \
"https://github.com/${AIRFLOW_REPO}/archive/${AIRFLOW_BRANCH}.tar.gz#egg=apache-airflow[${AIRFLOW_EXTRAS}]" \
- --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}" || true
+ --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}" \
+ || pip install 'pydantic>=2' || true
Review Comment:
Looks great. Yeah. Lets' remove it then and see if `pip` can resolve them on their own. If there are problems with "eager upgrade" and pip not finding the right resolution automatically, the right way to handle it will be to add "pydantic>=2" to `EAGER_UPGRADE_ADDITIONAL_REQUIREMENTS` (currently empty so it should be commented that likely we should remove it in the future when the resolution is improved) in both Dockerfile and Dockerfile.ci. This is the way how we "help" `pip` by additionally limiting the `eager upgrade`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #32366: Support Pydantic 2
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #32366:
URL: https://github.com/apache/airflow/pull/32366#discussion_r1255248464
##########
airflow/serialization/pydantic/dag_run.py:
##########
@@ -30,7 +29,7 @@ class DagRunPydantic(BaseModelPydantic):
id: int
dag_id: str
queued_at: Optional[datetime]
- execution_date: DateTime
+ execution_date: datetime
Review Comment:
Yeah I think they basically switch from `isinstance` to type identity check, probably for performance concerns (we have similar designs in our own serialisation code). The old behaviour can be brought back with a flag, but this doesn’t need that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #32366: Support Pydantic 2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #32366:
URL: https://github.com/apache/airflow/pull/32366#discussion_r1254269611
##########
airflow/serialization/pydantic/dag_run.py:
##########
@@ -30,7 +29,7 @@ class DagRunPydantic(BaseModelPydantic):
id: int
dag_id: str
queued_at: Optional[datetime]
- execution_date: DateTime
+ execution_date: datetime
Review Comment:
Ah... So Pydantic just got more "picky" about those. Cool.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lmrlyv commented on pull request #32366: Support Pydantic 2
Posted by "lmrlyv (via GitHub)" <gi...@apache.org>.
lmrlyv commented on PR #32366:
URL: https://github.com/apache/airflow/pull/32366#issuecomment-1684122950
> > I'm interested in finding out when the next release is expected to be made available. Having support for Pydantic 2 would be highly beneficial.
>
> Why? Could you explain why ?
>
> Airlfow 2.7.0 is being released today as we speak (working on it). But in constraints Pydantic is still < 2 - because some of our dependencies limit us, But we have no requiments, and if you are not using any provider that limits pydantic through their dependencies, you might attempt to upgrade and see if it works for you. But you will need to test it and make sure it works.
If you're asking about the reasons for using Pydantic v2, I'd highlight its significant speed improvement of 5-50 times, along with its enhanced stability and flexibility when compared to v1 (according to its [release notes](https://docs.pydantic.dev/latest/blog/pydantic-v2-alpha/)).
Regarding why we are waiting for this release, it's because we encountered an error (although I don't remember the details now) while attempting to deploy our airflow (v2.6.3) project that relies on Pydantic v2 models into a Kubernetes cluster. There was an open issue in this repo, and this pull request is expected to address and resolve that problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #32366: Support Pydantic 2
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #32366:
URL: https://github.com/apache/airflow/pull/32366#issuecomment-1623301111
Hey, the CI passes.
If I can get some reviews to ensure the changes are OK, I’ll add a commit to remove the build script hack so this is merge-able.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on a diff in pull request #32366: Support Pydantic 2
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #32366:
URL: https://github.com/apache/airflow/pull/32366#discussion_r1257324991
##########
setup.cfg:
##########
@@ -126,9 +126,7 @@ install_requires =
pendulum>=2.0
pluggy>=1.0
psutil>=4.2.0
- # Limit Pydantic to <2.0.0 because it breaks Kubernetes tests and building providers
- # This limit should be removed after https://github.com/apache/airflow/issues/32311 is fixed
- pydantic>=1.10.0,<2.0.0
+ pydantic>=1.10.0
Review Comment:
https://github.com/apache/airflow/pull/32444 will handle it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lmrlyv commented on pull request #32366: Support Pydantic 2
Posted by "lmrlyv (via GitHub)" <gi...@apache.org>.
lmrlyv commented on PR #32366:
URL: https://github.com/apache/airflow/pull/32366#issuecomment-1683947393
Hi, @potiuk!
I'm interested in finding out when the next release is expected to be made available. Having support for Pydantic 2 would be highly beneficial.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #32366: Support Pydantic 2
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #32366:
URL: https://github.com/apache/airflow/pull/32366#discussion_r1255247622
##########
Dockerfile:
##########
@@ -385,7 +385,8 @@ function install_airflow_dependencies_from_branch_tip() {
pip install --root-user-action ignore \
${ADDITIONAL_PIP_INSTALL_FLAGS} \
"https://github.com/${AIRFLOW_REPO}/archive/${AIRFLOW_BRANCH}.tar.gz#egg=apache-airflow[${AIRFLOW_EXTRAS}]" \
- --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}" || true
+ --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}" \
+ || pip install 'pydantic>=2' || true
Review Comment:
It would resolve it to 1.18 because (a dependency of) Moto pins it to `pydantic~=1.18`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #32366: Support Pydantic 2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #32366:
URL: https://github.com/apache/airflow/pull/32366#issuecomment-1684297639
> If you're asking about the reasons for using Pydantic v2, I'd highlight its significant speed improvement of 5-50 times, along with its enhanced stability and flexibility when compared to v1 (according to its [release notes](https://docs.pydantic.dev/latest/blog/pydantic-v2-alpha/)).
We are hardly using Pydantic, so improvements would be mariginal, but yes I am very well aware of it, just for "Airflow" it is not highly benefitial (yet). It might be in the future as we have plans for using Pydantic for internal API.
> Regarding why we are waiting for this release, it's because we encountered an error (although I don't remember the details now) while attempting to deploy our airflow (v2.6.3) project that relies on Pydantic v2 models into a Kubernetes cluster. There was an open issue in this repo, and this pull request is expected to address and resolve that problem.
Just checked: 2.7.0 stil has no support for Pydantic:
# We limit Pydantic to <2.0.0 until we can upgrade - there are limitation for Pydantic in AWS provider
# dependency (aws-sam-translator) - also we need to change orm-mode to from_attributes in definitions
# of the ORM models. See for the previous attempt https://github.com/apache/airflow/pull/33220
pydantic>=1.10.0,<2.0.0
That was likely an overlook as part of the problem with orm-mode at least is solved. So you will have to wait for 2.7.1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #32366: Support Pydantic 2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #32366:
URL: https://github.com/apache/airflow/pull/32366#issuecomment-1684318297
I added limitation removal and scheduled it for 2.7.1 https://github.com/apache/airflow/pull/33507 - seems that the aws-sam-translator removed the limitation last week and are going to be released in a week or two, which should be good as then our CI will test it automatically and we will produce constraints with Pydantic 2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #32366: Support Pydantic 2
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #32366:
URL: https://github.com/apache/airflow/pull/32366#discussion_r1254170810
##########
scripts/docker/install_airflow_dependencies_from_branch_tip.sh:
##########
@@ -51,7 +51,8 @@ function install_airflow_dependencies_from_branch_tip() {
pip install --root-user-action ignore \
${ADDITIONAL_PIP_INSTALL_FLAGS} \
"https://github.com/${AIRFLOW_REPO}/archive/${AIRFLOW_BRANCH}.tar.gz#egg=apache-airflow[${AIRFLOW_EXTRAS}]" \
- --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}" || true
+ --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}" \
+ || pip install 'pydantic>=2' || true
Review Comment:
This hack (and those in Dockerfile and Dockerfile.ci, those are auto-copied by pre-commit) will be removed before merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #32366: Support Pydantic 2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #32366:
URL: https://github.com/apache/airflow/pull/32366#issuecomment-1683964302
> I'm interested in finding out when the next release is expected to be made available. Having support for Pydantic 2 would be highly beneficial.
Why? Could you explain why ?
Airlfow 2.7.0 is being released today as we speak (working on it). But in constraints Pydantic is still < 2 - because some of our dependencies limit us, But we have no requiments, and if you are not using any provider that limits pydantic through their dependencies, you might attempt to upgrade and see if it works for you. But you will need to test it and make sure it works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #32366: Support Pydantic 2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #32366:
URL: https://github.com/apache/airflow/pull/32366
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on a diff in pull request #32366: Support Pydantic 2
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #32366:
URL: https://github.com/apache/airflow/pull/32366#discussion_r1257254709
##########
setup.cfg:
##########
@@ -126,9 +126,7 @@ install_requires =
pendulum>=2.0
pluggy>=1.0
psutil>=4.2.0
- # Limit Pydantic to <2.0.0 because it breaks Kubernetes tests and building providers
- # This limit should be removed after https://github.com/apache/airflow/issues/32311 is fixed
- pydantic>=1.10.0,<2.0.0
+ pydantic>=1.10.0
Review Comment:
Started to see
```
Unknown warnings generated:
/usr/local/lib/python3.8/site-packages/pydantic/_internal/_config.py:210:Support for class-based `config` is deprecated, use ConfigDict instead. Deprecated in Pydantic V2.0 to be removed in V3.0. See Pydantic V2 Migration Guide at https://errors.pydantic.dev/2.0.2/migration/
/usr/local/lib/python3.8/site-packages/pydantic/_internal/_config.py:261:Valid config keys have changed in V2: * 'orm_mode' has been renamed to 'from_attributes'
/usr/local/lib/python3.8/site-packages/pydantic/_internal/_config.py:210:Support for class-based `config` is deprecated, use ConfigDict instead. Deprecated in Pydantic V2.0 to be removed in V3.0. See Pydantic V2 Migration Guide at https://errors.pydantic.dev/2.0.2/migration/
/usr/local/lib/python3.8/site-packages/pydantic/_internal/_config.py:210:Support for class-based `config` is deprecated, use ConfigDict instead. Deprecated in Pydantic V2.0 to be removed in V3.0. See Pydantic V2 Migration Guide at https://errors.pydantic.dev/2.0.2/migration/
/usr/local/lib/python3.8/site-packages/pydantic/_internal/_config.py:210:Support for class-based `config` is deprecated, use ConfigDict instead. Deprecated in Pydantic V2.0 to be removed in V3.0. See Pydantic V2 Migration Guide at https://errors.pydantic.dev/2.0.2/migration/
/usr/local/lib/python3.8/site-packages/pydantic/_internal/_config.py:210:Support for class-based `config` is deprecated, use ConfigDict instead. Deprecated in Pydantic V2.0 to be removed in V3.0. See Pydantic V2 Migration Guide at https://errors.pydantic.dev/2.0.2/migration/
/usr/local/lib/python3.8/site-packages/pydantic/_internal/_config.py:210:Support for class-based `config` is deprecated, use ConfigDict instead. Deprecated in Pydantic V2.0 to be removed in V3.0. See Pydantic V2 Migration Guide at https://errors.pydantic.dev/2.0.2/migration/
/usr/local/lib/python3.8/site-packages/pydantic/_internal/_config.py:261:Valid config keys have changed in V2: * 'orm_mode' has been renamed to 'from_attributes'
ERROR! There were 8 warnings generated during the import
```
Example:
https://github.com/apache/airflow/actions/runs/5494206967/jobs/10012899407?pr=32441#step:11:1600
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org