Skip to content

Add HITLDetailHistory #55952

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.

Sign up for GitHub

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

Jump to bottom
Merged
Lee-W merged 4 commits into from
Oct 15, 2025
Merged

Add HITLDetailHistory #55952

Lee-W merged 4 commits into from
Oct 15, 2025

Conversation

@Lee-W
Copy link
Member

@Lee-W Lee-W commented Sep 22, 2025 •
edited
Loading

Why

Closes: #54956

What

  • Add HITLDetailHistory model
  • Record a HITLDetailHistory if TaskInstance has a HITLDetail when recording a TaskInstanceHistory
  • Add hitl_detail to get task_instance try and get task_instance tries APIs

^ 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 airflow-core/newsfragments.

@Lee-W Lee-W force-pushed the hitl-history branch 3 times, most recently from f87df88 to 681b9e9 Compare September 23, 2025 14:47
@Lee-W Lee-W marked this pull request as ready for review September 23, 2025 23:37
@Lee-W Lee-W changed the title feat(hitl): hitl history feat(hitl): Add HITLDetailHistory Sep 24, 2025
@Lee-W Lee-W moved this to In review in AIP-90 - Human in the loop Sep 24, 2025
jason810496
jason810496 reviewed Sep 25, 2025
View reviewed changes
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! LGTM overall.

@jason810496
Copy link
Member

jason810496 commented Sep 25, 2025 •
edited
Loading

I found that the test cases for the whole TaskInstanceHistory model are missing. Maybe we should cover the test cases in further PR. Or maybe adding another test method for HITLDetailHistory like

airflow/airflow-core/tests/unit/models/test_taskinstance.py

Lines 2665 to 2666 in 681b9e9

def test_task_instance_history_is_created_when_ti_goes_for_retry(self, dag_maker, session):
with dag_maker(serialized=True):
would be enough.

@Lee-W Lee-W force-pushed the hitl-history branch 3 times, most recently from bbaa9e2 to 4940d6e Compare September 28, 2025 06:02
@Lee-W
Copy link
Member Author

Lee-W commented Sep 28, 2025

I found that the test cases for the whole TaskInstanceHistory model are missing. Maybe we should cover the test cases in further PR. Or maybe adding another test method for HITLDetailHistory like

airflow/airflow-core/tests/unit/models/test_taskinstance.py

Lines 2665 to 2666 in 681b9e9

def test_task_instance_history_is_created_when_ti_goes_for_retry(self, dag_maker, session):
with dag_maker(serialized=True):

would be enough.

Thanks! Just added

jason810496
jason810496 reviewed Sep 30, 2025
View reviewed changes
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

jason810496
jason810496 reviewed Oct 6, 2025
View reviewed changes
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

LGTM overall, only a small final question

@ashb
Copy link
Member

ashb commented Oct 6, 2025

@Lee-W Please don't use "conventional commits" in PR titles (feat(hitl): etc) but instead either mark this with the label changelog-skip, or make the PR title a short readable sentence/statement.

@Lee-W Lee-W changed the title feat(hitl): Add HITLDetailHistory Add HITLDetailHistory Oct 8, 2025
@Lee-W Lee-W marked this pull request as draft October 8, 2025 19:24
@Lee-W Lee-W force-pushed the hitl-history branch 3 times, most recently from 7b7bcf7 to c5305ca Compare October 13, 2025 11:00
@Lee-W Lee-W marked this pull request as ready for review October 13, 2025 11:04
@Lee-W
Copy link
Member Author

Lee-W commented Oct 14, 2025

Hey @bbovenzi , would love to check with you before we merge this one. Thanks!

@Lee-W Lee-W merged commit b0421df into apache:main Oct 15, 2025
111 checks passed
@Lee-W Lee-W deleted the hitl-history branch October 15, 2025 09:15
@github-project-automation github-project-automation bot moved this from In review to Done in AIP-90 - Human in the loop Oct 15, 2025
pierrejeambrun
pierrejeambrun reviewed Oct 15, 2025
View reviewed changes
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

Nice!

snreddygopu pushed a commit to Teradata/airflow that referenced this pull request Oct 16, 2025
@Lee-W @sc250072
Add HITLDetailHistory (apache#55952)
0c456a5
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 17, 2025
@Lee-W
Add HITLDetailHistory (apache#55952)
0c71676
@Lee-W Lee-W mentioned this pull request Oct 17, 2025
Merged
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 19, 2025
@Lee-W
Add HITLDetailHistory (apache#55952)
1f85197
TyrellHaywood pushed a commit to TyrellHaywood/airflow that referenced this pull request Oct 22, 2025
@Lee-W @TyrellHaywood
Add HITLDetailHistory (apache#55952)
38c5147
@pierrejeambrun pierrejeambrun added this to the Airflow 3.2.0 milestone Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pierrejeambrun pierrejeambrun pierrejeambrun left review comments

@bbovenzi bbovenzi bbovenzi approved these changes

@jason810496 jason810496 jason810496 approved these changes

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy ephraimbuddy is a code owner

@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton ryanahamilton is a code owner

@jscheffl jscheffl Awaiting requested review from jscheffl

@shubhamraj-git shubhamraj-git Awaiting requested review from shubhamraj-git shubhamraj-git is a code owner

@bugraoz93 bugraoz93 Awaiting requested review from bugraoz93 bugraoz93 is a code owner

@potiuk potiuk Awaiting requested review from potiuk potiuk is a code owner

@rawwar rawwar Awaiting requested review from rawwar rawwar is a code owner

@XD-DENG XD-DENG Awaiting requested review from XD-DENG XD-DENG is a code owner

@ashb ashb Awaiting requested review from ashb ashb is a code owner

Assignees

No one assigned

Labels

None yet

Projects

AIP-90 - Human in the loop
Status: Done

Milestone

Airflow 3.2.0

Development

Successfully merging this pull request may close these issues.

Missing review history when tasks are cleared (HITL)

5 participants

@Lee-W @jason810496 @ashb @bbovenzi @pierrejeambrun