Skip to content

Conversation

@uranusjr
Copy link
Member

This does not work just yet because the example dags test is not set up correctly and cannot handle the SDK's DAG class.

WIP.

@kaxil
Copy link
Member

kaxil commented Mar 12, 2025

The following test failures should have an easy fix possibly

FAILED task-sdk/tests/task_sdk/definitions/test_asset_decorators.py::TestAssetDefinition::test__attrs_post_init__ - AssertionError: Expected 'DAG' to be called once. Called 0 times.

i.e. changing the following mock to SDK DAG

@mock.patch("airflow.models.dag.DAG")

@kaxil kaxil force-pushed the asset-deco-use-sdk-dag branch from fca337e to 450e888 Compare March 12, 2025 06:57
@kaxil
Copy link
Member

kaxil commented Mar 12, 2025

I just rebased the PR too to eliminate issues with default_view : #47616

@uranusjr
Copy link
Member Author

Yeah I think the most problematic part here is where we load example dags into DagBag for tests. Tests just send things in that DagBag directly into the scheduler, but the scheduler expects the DAG class in airflow.models instead.

There are a couple of other failures but they are all pretty trivial like the one you mentioned.

Relatedly, we currently do not have example DAGs that use the SDK. This would have been surfaced sooner if we added at least one.

@uranusjr uranusjr force-pushed the asset-deco-use-sdk-dag branch from 450e888 to e67b652 Compare March 12, 2025 10:27
@kaxil
Copy link
Member

kaxil commented Mar 12, 2025

Relatedly, we currently do not have example DAGs that use the SDK. This would have been surfaced sooner if we added at least one.

Yeah we have some here https://github.com/apache/airflow/tree/main/task-sdk/tests/task_sdk/dags but we have not changed all the example_dags in the https://github.com/apache/airflow/tree/main/airflow/example_dags folder

@ashb ashb force-pushed the asset-deco-use-sdk-dag branch from e67b652 to dd5c3f7 Compare March 12, 2025 21:02
@ashb ashb force-pushed the asset-deco-use-sdk-dag branch 2 times, most recently from 467ca8e to 76956d5 Compare March 13, 2025 11:44
@ashb ashb moved this from Todo to In Progress in AIP-72 - Task Execution Interface and SDK Mar 17, 2025
@ashb ashb force-pushed the asset-deco-use-sdk-dag branch 2 times, most recently from 9c59b13 to 4921932 Compare March 17, 2025 10:00
@ashb ashb force-pushed the asset-deco-use-sdk-dag branch from 4921932 to 0225a06 Compare April 1, 2025 13:58
@kaxil kaxil force-pushed the asset-deco-use-sdk-dag branch from 0225a06 to 67bc81f Compare April 4, 2025 20:54
This does not work just yet because the example dags test is not set up
correctly and cannot handle the SDK's DAG class.
@kaxil kaxil force-pushed the asset-deco-use-sdk-dag branch from 67bc81f to 0126850 Compare April 7, 2025 12:35
@kaxil kaxil marked this pull request as ready for review April 7, 2025 12:35
@kaxil kaxil requested review from amoghrajesh, ashb and kaxil as code owners April 7, 2025 12:35
@kaxil
Copy link
Member

kaxil commented Apr 7, 2025

This should now work!

@kaxil
Copy link
Member

kaxil commented Apr 7, 2025

unrelated failures

@kaxil kaxil merged commit 1481009 into apache:main Apr 7, 2025
64 of 93 checks passed
@kaxil kaxil deleted the asset-deco-use-sdk-dag branch April 7, 2025 14:04
simonprydden pushed a commit to simonprydden/airflow that referenced this pull request Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants