Move Asset user facing components to task_sdk #43773
Merged
+902
−733
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boring-cyborg
bot
added
area:API
Lee-W
force-pushed
the
move-asset-to-task-sdk
branch
6 times, most recently
from
c7ecee5 to
5c8d6e2
Compare
Lee-W
requested review from
bolkedebruin,
potiuk,
ashb,
jedcunningham,
mobuchowski,
eladkal,
o-nikolas,
uranusjr,
ryanahamilton,
bbovenzi,
pierrejeambrun,
jscheffl,
ephraimbuddy and
XD-DENG
as code owners
…on to return compat method
…age" This reverts commit 324efc0.
…ecorators to airflow.sdk.definitions.asset.*
Lee-W
force-pushed
the
move-asset-to-task-sdk
branch
from
9331e85 to
93ab795
Compare
uranusjr
approved these changes
Nov 20, 2024
eladkal
reviewed
Dec 3, 2024
Comment on lines
-26
to
42
| import packaging.version | ||
| from packaging.version import Version | ||
|
|
||
| from airflow import __version__ as airflow_version | ||
| from airflow import __version__ as AIRFLOW_VERSION | ||
|
|
||
| __all__ = ["__version__"] | ||
|
|
||
| __version__ = "1.2.2" | ||
|
|
||
| if packaging.version.parse(packaging.version.parse(airflow_version).base_version) < packaging.version.parse( | ||
| "2.8.0" | ||
| ): | ||
|
|
||
| AIRFLOW_V_3_0_PLUS = Version(Version(AIRFLOW_VERSION).base_version) >= Version("3.0.0") | ||
| AIRFLOW_V_2_10_PLUS = Version(Version(AIRFLOW_VERSION).base_version) >= Version("2.10.0") | ||
| AIRFLOW_V_2_9_PLUS = Version(Version(AIRFLOW_VERSION).base_version) >= Version("2.9.0") | ||
| AIRFLOW_V_2_8_PLUS = Version(Version(AIRFLOW_VERSION).base_version) >= Version("2.8.0") | ||
|
|
||
| if not AIRFLOW_V_2_8_PLUS: | ||
| raise RuntimeError( | ||
| f"The package `apache-airflow-providers-common-compat:{__version__}` needs Apache Airflow 2.8.0+" |
There was a problem hiding this comment.
This will cause same problem as https://github.com/apache/airflow/pull/43556/files#r1841784583
It will be overridden during release time of providers.
We probably need to find time to complete #44024 to avoid more of this problem
There was a problem hiding this comment.
Agree. Plus - as discussed in slack https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1733228100524949 we should aalso not reuse the same constant names for those versions in providers, they are used and defined in tests_common and it's very easy to confuse them.
my proposal will be to:
- use
TEST_AIRFLOW_Vin tests - add those constants
AIRFLOW_V`` to common.compat in "versions" package (not ininit.py`) - bump min version of common for all providers to make sure all of them have it
That should solve the problem permanently and in a few places where some providers already use their own constants - we could use them from common.compat (for example here https://github.com/apache/airflow/blob/main/providers/src/airflow/providers/amazon/aws/assets/s3.py#L40)
WDYT @uranusjr @dstandish @eladkal ? Does it sound good?
There was a problem hiding this comment.
I moved them to verions in #44610
LefterisXefteris
pushed a commit
to LefterisXefteris/airflow
that referenced
this pull request
Jan 5, 2025
* feat(task_sdk): Move asset from Airflow core to task_sdk * feat(task_sdk): Move assets.metadata to task_sdk.definitions.asset * fix(providers/amazon): fix common.compat provider ImportError handling * fix(providers/google): fix common.compat provider ImportError handling * fix(providers/openlineage): fix common.compat provider ImportError handling * fix(provider/common/compat): fix common.compat provider ImportError handling * feat(task_sdk): expose Model * docs(nesfragements): update how asset module should be imported * fix(task_sdk): fix 2_10 compatibility * feat(common.compat): use version to decide how to import assets instead of exception * feat(providers/common.compat): use airflow version instead of exception to return compat method * refactor(providers/common/compat): extract airflow version to __init__ * fix(providers): use version compare to decide whether to import asset * feat(decorators/asset): move @asset to task_sdk * refactor(asset): rename _AssetAliasCondition as AssetAliasCondition * feat(task_sdk): make airflow.sdk.definitions.decoratos a package * Revert "feat(task_sdk): make airflow.sdk.definitions.decoratos a package" This reverts commit 324efc0. * feat(task_sdk): move asset related logic in airflow.sdk.definitions.decorators to airflow.sdk.definitions.asset.* * refactor(task_sdk): move @asset to airflow.sdk.definitions.asset.decorators * test(providers/amazon): remove unnecessary compat handling * test(providers/google): remove unnecessary compat handling * test(openlineage): remove unnecessary compat handling * fix(provider/openlineage): fix how asset compat is handled * feat(task_sdk/asset): expose extract_event_key * test(providers/google): change Asset import back to common.compat * docs(newsfragments): fix error naming * docs(newsfragments): fix typo * docs(newsfragment): add missing metadata * fixup! feat(task_sdk): Move asset from Airflow core to task_sdk * fixup! feat(task_sdk): Move asset from Airflow core to task_sdk
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
As part of AIP-72
Closes: #43619
What
^ 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.rstor{issue_number}.significant.rst, in newsfragments.