Skip to content

Conversation

@Lee-W
Copy link
Member

@Lee-W Lee-W commented Nov 7, 2024

Why

As part of AIP-72

Closes: #43619

What

  • Move "Asset", "AssetAll", "AssetAny", "Dataset", "Model", "@asset" from Airflow Core to task_sdk
  • Import Asset from task_sdk for common.compat provider
  • Fix ImportError handling in common.compat provider
    • Add a temporary workaround for this error for the following providers (which should be removed after common.compat provider change in this PR has been released)
      • providers/openlineage
      • providers/google
      • providers/amazon

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

@Lee-W Lee-W added the legacy api Whether legacy API changes should be allowed in PR label Nov 7, 2024
@Lee-W Lee-W force-pushed the move-asset-to-task-sdk branch 6 times, most recently from c7ecee5 to 5c8d6e2 Compare November 7, 2024 10:02
@Lee-W Lee-W marked this pull request as ready for review November 7, 2024 11:40
@Lee-W Lee-W force-pushed the move-asset-to-task-sdk branch from 9331e85 to 93ab795 Compare November 20, 2024 07:43
@Lee-W Lee-W merged commit a0f3353 into apache:main Nov 20, 2024
1 check passed
@Lee-W Lee-W deleted the move-asset-to-task-sdk branch November 20, 2024 09:09
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+"
Copy link
Contributor

@eladkal eladkal Dec 3, 2024

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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_V in tests
  • add those constants AIRFLOW_V`` to common.compat in "versions" package (not in init.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?

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved them to verions in #44610

got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 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

Labels

airflow3.0:breaking Candidates for Airflow 3.0 that contain breaking changes area:API Airflow's REST/HTTP API area:lineage area:providers area:Scheduler including HA (high availability) scheduler area:serialization area:task-execution-interface-aip72 AIP-72: Task Execution Interface (TEI) aka Task SDK area:webserver Webserver related Issues kind:documentation legacy api Whether legacy API changes should be allowed in PR provider:common-compat

Development

Successfully merging this pull request may close these issues.

Move Asset user-facing components to task_sdk

4 participants