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

Set logical_date and data_interval to None for asset-triggered dags and forbid them to be accessed in context/template #46460

Conversation

Lee-W
Copy link
Member

@Lee-W Lee-W commented Feb 5, 2025

Why

closes: #46192, #46193, #46194, #46196, #46198, #46641

What

  • Set logical_date and data_interval to None for asset-triggered dags.
  • Not adding data_interval_start, data_interval_end, prev_data_interval_start_success, prev_data_interval_end_success if data_interval_start and data_interval_end is None

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

@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Feb 5, 2025
@Lee-W
Copy link
Member Author

Lee-W commented Feb 5, 2025

blocked by #46295 and #46398

@Lee-W Lee-W added the AIP-83 Remove Execution Date Unique Constraint from DAG Run label Feb 5, 2025
@Lee-W Lee-W force-pushed the none-as-logical-date-and-data-interval-for-asset-triggered-dag branch 2 times, most recently from 77e97e1 to 82081f9 Compare February 10, 2025 09:56
@Lee-W
Copy link
Member Author

Lee-W commented Feb 10, 2025

Functionality-wise, it works ok for now. I'll take a look at other PRs to confirm whether the UI parts are blocked by other PR or something I should fixed in this one

@Lee-W Lee-W marked this pull request as ready for review February 10, 2025 14:41
@Lee-W Lee-W requested review from XD-DENG and ashb as code owners February 10, 2025 14:41
@Lee-W Lee-W force-pushed the none-as-logical-date-and-data-interval-for-asset-triggered-dag branch from dc45928 to 1a82214 Compare February 10, 2025 14:41
@Lee-W Lee-W requested a review from uranusjr as a code owner February 10, 2025 14:41
@Lee-W
Copy link
Member Author

Lee-W commented Feb 10, 2025

The dag and task can be correctly triggered and finished. However, the UI can not find the correct dag. Not sure whether it's due to #46616

image

it looks like this now.

any idea? @uranusjr

@Lee-W Lee-W changed the title None as logical date and data interval for asset triggered dag Set logical_date and data_interval to None for asset-triggered dags Feb 10, 2025
@uranusjr
Copy link
Member

the UI can not find the correct dag

Can you elaborate? I don’t have enough context to notice what’s wrong in the screenshot.

@Lee-W
Copy link
Member Author

Lee-W commented Feb 11, 2025

the UI can not find the correct dag

Can you elaborate? I don’t have enough context to notice what’s wrong in the screenshot.

Sure, the second dag asset_producer_2 has some success tasks but no dags on the UI.

@Lee-W Lee-W force-pushed the none-as-logical-date-and-data-interval-for-asset-triggered-dag branch 4 times, most recently from 5a32895 to 49a250c Compare February 11, 2025 13:17
@Lee-W Lee-W force-pushed the none-as-logical-date-and-data-interval-for-asset-triggered-dag branch from 49a250c to 7e3cde7 Compare February 11, 2025 13:54
@Lee-W Lee-W force-pushed the none-as-logical-date-and-data-interval-for-asset-triggered-dag branch from 7e3cde7 to 08f8991 Compare February 11, 2025 14:59
@Lee-W Lee-W requested a review from potiuk as a code owner February 11, 2025 14:59
@Lee-W Lee-W changed the title Set logical_date and data_interval to None for asset-triggered dags Set logical_date and data_interval to None for asset-triggered dags and forbid these value to be access in context/tempalte Feb 11, 2025
Lee-W added 14 commits February 12, 2025 19:40
…ata_interval_start_success, prev_data_interval_end_success for dag_run that has no data_interval
…ogical_date"

This reverts commit a8be4a7ebb424b194c8dd3183243b2516e4edb66.
This reverts commit a1b4e6e8299b3e0e800db088bc95b30519fe6b1a.
… logic as we don't need to check existing logical_date for asset triggered dag runs
@Lee-W Lee-W force-pushed the none-as-logical-date-and-data-interval-for-asset-triggered-dag branch from 0cc1160 to b8660f8 Compare February 12, 2025 11:41
@Lee-W
Copy link
Member Author

Lee-W commented Feb 12, 2025

@uranusjr The CI is green now 🙌

@Lee-W Lee-W requested a review from uranusjr February 12, 2025 13:11
@Lee-W
Copy link
Member Author

Lee-W commented Feb 12, 2025

It's a bit late in our time zone now.

@dstandish, it would be nice if you could take a look tomorrow if you have some bandwidth. Thanks!

@uranusjr
Copy link
Member

Looks good to me aside from the one question above

@Lee-W Lee-W merged commit 8dd900b into apache:main Feb 13, 2025
91 checks passed
@Lee-W Lee-W deleted the none-as-logical-date-and-data-interval-for-asset-triggered-dag branch February 13, 2025 07:42
@Lee-W
Copy link
Member Author

Lee-W commented Feb 13, 2025

I thought merging it would close all the six issues 🤔 maybe the syntax is wrong, I'll close the rest 5 manually

ambika-garg pushed a commit to ambika-garg/airflow that referenced this pull request Feb 17, 2025
…nd forbid them to be accessed in context/template (apache#46460)

* style(dag): improve type annotation

* refactor(dag): rename by_dag as adrq_by_dag

* feat(scheduler_job_runner): set logical_date, data_interval as none when creating dag runs for asset triggered dag

* test(pytest_plugin): set run_after to now if data_interval is None

* test(scheduler_job_runner): set logical_date and data_interval of asset triggered dag runs to none

* feat(dagrun): order queued and running dag runs by run_after instead of logical_date

* feat(dag): get task_instances based on run_after instead of logical_date

* feat(taskinstance): change log_url base_date to use run_after instead of logical_date

* test(test_common): rewrite create_dagrun as logical_date is now nullable

* feat(dag): get the last_dagrun by run_after

* feat(taskinstance): pass base_date to TaskInstance.log_uri only when logical_date exists

* feat(www): fix last_dag_run through run_after

* feat(www): fetch last_dag_runs using run_after instead of logical_date in task_stat

* feat(www): fetch dag_run through run_after instead of logical_date in grid_data

* feat(task_sdk): remove data_interval_start, data_interval_end, prev_data_interval_start_success, prev_data_interval_end_success for dag_run that has no data_interval

* test(pytest_plugin): add DagRun.DATASET_TRIGERED for backward compat

* Revert "test(test_common): rewrite create_dagrun as logical_date is now nullable"

This reverts commit a3ba5a1.

* Revert "feat(dag): get task_instances based on run_after instead of logical_date"

This reverts commit a8be4a7ebb424b194c8dd3183243b2516e4edb66.

* Revert "feat(dag): get the last_dagrun by run_after"

This reverts commit a1b4e6e8299b3e0e800db088bc95b30519fe6b1a.

* feat(timetable): remove AssetTriggeredTimetable.data_interval_for_events

* feat(scheduler_job_runner): use start_date directly for asset triggered dag

* Revert "feat(www): fetch dag_run through run_after instead of logical_date in grid_data"

This reverts commit 24a79d6.

* Revert "feat(www): fetch last_dag_runs using run_after instead of logical_date in task_stat"

This reverts commit 0f04880.

* Revert "feat(www): fix last_dag_run through run_after"

This reverts commit 5c90113.

* refactor(task_runner): merge the data_interval keys with logical_date check logic

* feat(scheduler_job_runner): simplify _create_dag_runs_asset_triggered logic as we don't need to check existing logical_date for asset triggered dag runs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AIP-83 Remove Execution Date Unique Constraint from DAG Run area:Scheduler including HA (high availability) scheduler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AIP-83 2.a. Asset-triggered dag runs will not have a logical date or a data interval
3 participants