Skip to content

Fix stray order_by(TaskInstance.execution_date) - #21705

Merged
uranusjr merged 1 commit into
apache:mainfrom
astronomer:avoid-order-by-ti-execution-date
Feb 22, 2022
Merged

uranusjr merged 1 commit into
apache:mainfrom
astronomer:avoid-order-by-ti-execution-date

Conversation

@uranusjr

Copy link
Copy Markdown
Member

Fix #21656.

Nobody noticed this exists because the function BaseOperator.get_task_instances() is not used anywhere in the code base. But since this is public API, we should still fix it.

A quick search for order_by(TaskInstance.execution_date) and order_by(TI.execution_date) yielded no other results; this is probably the only one we missed.

@uranusjr uranusjr added this to the Airflow 2.2.5 milestone Feb 21, 2022
@github-actions

Copy link
Copy Markdown
Contributor

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions Bot added the full tests needed We need to run full set of tests for this PR to merge label Feb 21, 2022
@TidesMind

Copy link
Copy Markdown

Thank you for fixing this!

@uranusjr
uranusjr force-pushed the avoid-order-by-ti-execution-date branch from e96650b to 6497ffb Compare February 22, 2022 18:27
@uranusjr
uranusjr merged commit bb577a9 into apache:main Feb 22, 2022
@uranusjr
uranusjr deleted the avoid-order-by-ti-execution-date branch February 22, 2022 20:12
@ashb

ashb commented Feb 23, 2022 •

Copy link
Copy Markdown
Member

Change looks good, but I thought this would work through theh association_proxy anyway?

Obviously not by the error report

@uranusjr

uranusjr commented Feb 23, 2022 •

Copy link
Copy Markdown
Member Author

filter() works, but not order_by(). There were other occurrences I fixed before 2.2 on this. It’s quite annoying (I would’ve said surprising but I know too much about SQLAlchemy to be surprised).

@jedcunningham jedcunningham added the type:bug-fix Changelog: Bug Fixes label Feb 28, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 16, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 20, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 22, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 22, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 22, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 22, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 24, 2022
ephraimbuddy pushed a commit that referenced this pull request Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full tests needed We need to run full set of tests for this PR to merge type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Airflow >= 2.2.0 execution date change is failing TaskInstance get_task_instances method and possibly others

5 participants