Skip to content

Conversation

@sunank200
Copy link
Collaborator

@sunank200 sunank200 commented Feb 10, 2025

This PR started with changes on #46398. As the logic on 46398 has changed, I have created a different PR and closed the other PR

For the AIP 83 amendment, when a null logical date is provided, run_id is run_after + random string.

Closes: #46199


^ 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 area:API Airflow's REST/HTTP API area:core-operators area:Scheduler including HA (high availability) scheduler area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Feb 10, 2025
@sunank200 sunank200 force-pushed the run_id_logic_with_no_logical_date branch 3 times, most recently from 6776668 to 25cfbf8 Compare February 10, 2025 11:31
@sunank200 sunank200 added the legacy api Whether legacy API changes should be allowed in PR label Feb 10, 2025
@sunank200 sunank200 requested a review from Lee-W February 10, 2025 11:51
@sunank200 sunank200 force-pushed the run_id_logic_with_no_logical_date branch from 175ad1f to fd91711 Compare February 10, 2025 12:53
@sunank200 sunank200 added the AIP-83 Remove Execution Date Unique Constraint from DAG Run label Feb 10, 2025
@phanikumv phanikumv requested a review from dstandish February 10, 2025 13:36
@sunank200 sunank200 requested a review from uranusjr February 10, 2025 14:46
@sunank200 sunank200 force-pushed the run_id_logic_with_no_logical_date branch 9 times, most recently from a351015 to 9b2116e Compare February 10, 2025 17:24
@sunank200 sunank200 force-pushed the run_id_logic_with_no_logical_date branch 2 times, most recently from e6ccc9f to 2a4dc29 Compare February 11, 2025 09:48
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

Two minor issues in test set up, otherwise this should be ready.

@sunank200 sunank200 force-pushed the run_id_logic_with_no_logical_date branch from 2a4dc29 to 5470414 Compare February 11, 2025 10:21
@sunank200
Copy link
Collaborator Author

Two minor issues in test set up, otherwise this should be ready.

@uranusjr these should be fixed now.

@phanikumv phanikumv merged commit 035060d into apache:main Feb 11, 2025
91 checks passed
@phanikumv phanikumv deleted the run_id_logic_with_no_logical_date branch February 11, 2025 11:40
@dstandish
Copy link
Contributor

I think we need to make run_after non public. Feels like we should not expose this to the user as an option -- but rather, it's something that the scheduler should figure out based on the dag / dag run.

dag_bag: DagBag,
*,
triggered_by: DagRunTriggeredByType,
run_after: datetime,
Copy link
Contributor

Choose a reason for hiding this comment

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

so like e.g. this, not sure it makes sense to have this param

Copy link
Contributor

@dstandish dstandish Feb 11, 2025

Choose a reason for hiding this comment

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

but basically, any public function or public endpoint, my thought is, probably we should not expose this to user as an option / param. essentially it's read only attr i'm thinking.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created an issue here to fix this: #46650

ambika-garg pushed a commit to ambika-garg/airflow that referenced this pull request Feb 17, 2025
…s None. (apache#46616)

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
NilsJPWerner added a commit to NilsJPWerner/airflow that referenced this pull request Oct 3, 2025
In Airflow 3.x, commit 035060d (PR apache#46616) changed the trigger_dag
module to use DagRun.generate_run_id() instead of
dag.timetable.generate_run_id() for manual DAG runs. This bypassed
custom timetable logic, causing a regression from Airflow 2.x behavior.

This commit restores the use of dag.timetable.generate_run_id() for
manual triggers, allowing custom timetables to control run_id generation
for both scheduled and manually triggered runs.

Changes:
- airflow/api/common/trigger_dag.py: Changed to call
  dag.timetable.generate_run_id() with run_after and data_interval
  parameters instead of DagRun.generate_run_id()
- tests/api_fastapi/core_api/routes/public/test_dag_run.py: Added test
  to verify custom timetable generate_run_id is called for manual
  triggers with both logical_date provided and null

This pattern matches how manual triggers are handled in other parts of
the codebase (e.g., assets.py, scheduler_job_runner.py).

Fixes: apache#55908
NilsJPWerner added a commit to NilsJPWerner/airflow that referenced this pull request Oct 3, 2025
In Airflow 3.x, commit 035060d (PR apache#46616) changed the trigger_dag
module to use DagRun.generate_run_id() instead of
dag.timetable.generate_run_id() for manual DAG runs. This bypassed
custom timetable logic, causing a regression from Airflow 2.x behavior.

This commit restores the use of dag.timetable.generate_run_id() for
manual triggers, allowing custom timetables to control run_id generation
for both scheduled and manually triggered runs.

Changes:
- airflow/api/common/trigger_dag.py: Changed to call
  dag.timetable.generate_run_id() with run_after and data_interval
  parameters instead of DagRun.generate_run_id()
- tests/api_fastapi/core_api/routes/public/test_dag_run.py: Added test
  to verify custom timetable generate_run_id is called for manual
  triggers with both logical_date provided and null

This pattern matches how manual triggers are handled in other parts of
the codebase (e.g., assets.py, scheduler_job_runner.py).

Fixes: apache#55908
pierrejeambrun pushed a commit to NilsJPWerner/airflow that referenced this pull request Oct 14, 2025
In Airflow 3.x, commit 035060d (PR apache#46616) changed the trigger_dag
module to use DagRun.generate_run_id() instead of
dag.timetable.generate_run_id() for manual DAG runs. This bypassed
custom timetable logic, causing a regression from Airflow 2.x behavior.

This commit restores the use of dag.timetable.generate_run_id() for
manual triggers, allowing custom timetables to control run_id generation
for both scheduled and manually triggered runs.

Changes:
- airflow/api/common/trigger_dag.py: Changed to call
  dag.timetable.generate_run_id() with run_after and data_interval
  parameters instead of DagRun.generate_run_id()
- tests/api_fastapi/core_api/routes/public/test_dag_run.py: Added test
  to verify custom timetable generate_run_id is called for manual
  triggers with both logical_date provided and null

This pattern matches how manual triggers are handled in other parts of
the codebase (e.g., assets.py, scheduler_job_runner.py).

Fixes: apache#55908
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:API Airflow's REST/HTTP API area:Scheduler including HA (high availability) scheduler area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues legacy api Whether legacy API changes should be allowed in PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-83 question 6 run_id logic when no logical date

5 participants