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

Turn Pydantic into an optional dependency #37320

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Feb 10, 2024

We've been internally using pydantic for internal API and it caused some compatibility issues, because Pydantic is so popular and currently still users of Pydantic are somewhat split between Pydantic 1 and Pydantic 2. The popularity of Pydantic works against us, and since we are not yet using it in "production" (and in the future we will only actually use it for Internal API), it seems that turning Pydantic into an optional dependency is the best way we can proceed.

It's as simple as converting all the direct imports into a common util imports that have a fallback mechanism when import is not found.

This should enable less conflicts when installing 3rd-party libraries with Airflow.


^ 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 added this to the Airflow 2.8.2 milestone Feb 10, 2024
@potiuk potiuk force-pushed the make-pydantic-optional-dependency branch 4 times, most recently from 312f55d to ac41af9 Compare February 11, 2024 01:09
@potiuk potiuk changed the title Turn Pydantic into an optional depenedency Turn Pydantic into an optional dependency Feb 11, 2024
@potiuk potiuk force-pushed the make-pydantic-optional-dependency branch from ac41af9 to 676a4ec Compare February 11, 2024 01:54
@potiuk potiuk force-pushed the make-pydantic-optional-dependency branch from 676a4ec to 88c435a Compare February 11, 2024 01:57
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@potiuk potiuk force-pushed the make-pydantic-optional-dependency branch 3 times, most recently from 0f41e34 to b9b06a1 Compare February 11, 2024 21:10
We've been internally using pydantic for internal API and it caused
some compatibility issues, because Pydantic is so popular and currently
still users of Pydantic are somewhat split between Pydantic 1 and
Pydantic 2.  The popularity of Pydantic works against us, and since we
are not yet using it in "production" (and in the future we will only
actually use it for Internal API), it seems that turning Pydantic into
an optional dependency is the best way we can proceed.

It's as simple as converting all the direct imports into a common util
imports that have a fallback mechanism when import is not found.

This should enable less conflicts when installing 3rd-party libraries
with Airflow.

Added test where pydantic is removed. Also made sure that the special
cases we have tests for run full suite of tests - non-db and db.
@potiuk potiuk force-pushed the make-pydantic-optional-dependency branch from 3d6128a to 16aeb19 Compare February 12, 2024 20:42
@potiuk potiuk merged commit c3f48ee into apache:main Feb 12, 2024
84 checks passed
@potiuk potiuk deleted the make-pydantic-optional-dependency branch February 12, 2024 23:08
potiuk added a commit that referenced this pull request Feb 13, 2024
We've been internally using pydantic for internal API and it caused
some compatibility issues, because Pydantic is so popular and currently
still users of Pydantic are somewhat split between Pydantic 1 and
Pydantic 2.  The popularity of Pydantic works against us, and since we
are not yet using it in "production" (and in the future we will only
actually use it for Internal API), it seems that turning Pydantic into
an optional dependency is the best way we can proceed.

It's as simple as converting all the direct imports into a common util
imports that have a fallback mechanism when import is not found.

This should enable less conflicts when installing 3rd-party libraries
with Airflow.

Added test where pydantic is removed. Also made sure that the special
cases we have tests for run full suite of tests - non-db and db.

(cherry picked from commit c3f48ee)
potiuk added a commit to potiuk/airflow that referenced this pull request Feb 13, 2024
The changes in apache#37320 introduced flakiness while removing side-effect
of configuration test. Cleanup of ProvidersManager might
interfere with other tests using it at the same time. This caused
some Non-DB test failures with missing keys in ProvidersManager.

The fix is to mark the configuration test as DB-test - then it
will never be run by NonDB xdist and it will always run
sequentially to other tests (and still it will not
introduce side-effect as cleanup will always be done between tests.
potiuk added a commit that referenced this pull request Feb 13, 2024
The changes in #37320 introduced flakiness while removing side-effect
of configuration test. Cleanup of ProvidersManager might
interfere with other tests using it at the same time. This caused
some Non-DB test failures with missing keys in ProvidersManager.

The fix is to mark the configuration test as DB-test - then it
will never be run by NonDB xdist and it will always run
sequentially to other tests (and still it will not
introduce side-effect as cleanup will always be done between tests.
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Feb 19, 2024
ephraimbuddy pushed a commit that referenced this pull request Feb 20, 2024
The changes in #37320 introduced flakiness while removing side-effect
of configuration test. Cleanup of ProvidersManager might
interfere with other tests using it at the same time. This caused
some Non-DB test failures with missing keys in ProvidersManager.

The fix is to mark the configuration test as DB-test - then it
will never be run by NonDB xdist and it will always run
sequentially to other tests (and still it will not
introduce side-effect as cleanup will always be done between tests.

(cherry picked from commit cbc9af0)
ephraimbuddy pushed a commit that referenced this pull request Feb 22, 2024
We've been internally using pydantic for internal API and it caused
some compatibility issues, because Pydantic is so popular and currently
still users of Pydantic are somewhat split between Pydantic 1 and
Pydantic 2.  The popularity of Pydantic works against us, and since we
are not yet using it in "production" (and in the future we will only
actually use it for Internal API), it seems that turning Pydantic into
an optional dependency is the best way we can proceed.

It's as simple as converting all the direct imports into a common util
imports that have a fallback mechanism when import is not found.

This should enable less conflicts when installing 3rd-party libraries
with Airflow.

Added test where pydantic is removed. Also made sure that the special
cases we have tests for run full suite of tests - non-db and db.

(cherry picked from commit c3f48ee)
ephraimbuddy pushed a commit that referenced this pull request Feb 22, 2024
The changes in #37320 introduced flakiness while removing side-effect
of configuration test. Cleanup of ProvidersManager might
interfere with other tests using it at the same time. This caused
some Non-DB test failures with missing keys in ProvidersManager.

The fix is to mark the configuration test as DB-test - then it
will never be run by NonDB xdist and it will always run
sequentially to other tests (and still it will not
introduce side-effect as cleanup will always be done between tests.

(cherry picked from commit cbc9af0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers area:serialization area:system-tests kind:documentation provider:google Google (including GCP) related issues provider:papermill type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants