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

Update updated_at when saving TI to DB #40782

Merged
merged 3 commits into from
Aug 2, 2024
Merged

Conversation

gyli
Copy link
Contributor

@gyli gyli commented Jul 14, 2024

fixes: #40623

Tested with query

select start_date, end_date, updated_at from task_instance where task_id = 'sleep' ORDER BY start_date DESC;

The lower result is the initial test with example provided in the issue (sleep time is reduced from 50 to 5), showing updated_at is before end_date.
The upper result is the log after this fix, showing updated_at is now after end_date.
Screenshot 2024-07-14 at 6 02 47 PM

I have also tried flag_modified, while it does not work at the same place.


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

@gyli gyli requested review from kaxil, XD-DENG and ashb as code owners July 14, 2024 22:08
@shahar1 shahar1 changed the title fix: update updated_at when saving to db as session.merge does not tr… Update updated_at when saving TI to DB Jul 15, 2024
@eladkal eladkal added this to the Airflow 2.9.4 milestone Jul 16, 2024
@eladkal eladkal added the type:bug-fix Changelog: Bug Fixes label Jul 16, 2024
@gyli
Copy link
Contributor Author

gyli commented Jul 18, 2024

Hi @shahar1 and @romsharon98, could you help me unblock the failed check here? Does it matter for merging PR?
The failed unit tests seem unrelated to my change, and they both pass on my local when I do breeze shell --force-lowest-dependencies and then pytest tests/triggers/test_external_task.py.

@shahar1
Copy link
Contributor

shahar1 commented Aug 2, 2024

Hi @shahar1 and @romsharon98, could you help me unblock the failed check here? Does it matter for merging PR? The failed unit tests seem unrelated to my change, and they both pass on my local when I do breeze shell --force-lowest-dependencies and then pytest tests/triggers/test_external_task.py.

I apologize as it seems like I've missed your last message.
I think that we're good to go :)

@shahar1 shahar1 merged commit 5880545 into apache:main Aug 2, 2024
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

updated_at column in task_instance table not updating correctly
5 participants