Skip to content

Conversation

@kaxil
Copy link
Member

@kaxil kaxil commented Oct 30, 2025

When a task doesn't specify an executor, the executor field in the database remained NULL, causing it to not display in the UI. This fix resolves the executor to the default configured executor name at task instance creation and refresh time, following the same pattern as other fields like pool and queue.

The fix modifies TaskInstance.insert_mapping() and TaskInstance.refresh_from_task() to automatically populate the executor field with the default executor when task.executor is None, ensuring the field always displays correctly in the UI.

closes #57526

Same screenshots as #57526 :)


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

@kaxil
Copy link
Member Author

kaxil commented Oct 30, 2025

cc @dheerajturaga

Copy link
Contributor

@dheerajturaga dheerajturaga left a comment

Choose a reason for hiding this comment

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

Awesome! works like a charm!

@kaxil kaxil added this to the Airflow 3.1.2 milestone Oct 31, 2025
@kaxil kaxil requested a review from jscheffl October 31, 2025 00:19
When a task doesn't specify an executor, the executor field in the database
remained `NULL`, causing it to not display in the UI. This fix resolves the
executor to the default configured executor name at task instance creation
and refresh time, following the same pattern as other fields like pool and
queue.

The fix modifies TaskInstance.insert_mapping() and TaskInstance.refresh_from_task()
to automatically populate the executor field with the default executor when
task.executor is None, ensuring the field always displays correctly in the UI.

closes apache#57526
@kaxil kaxil force-pushed the ensure-executor-exists branch from c7be733 to e7b5533 Compare October 31, 2025 00:38
Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Looks good, one sanity question


executor = task.executor
if executor is None:
executor_name = ExecutorLoader.get_default_executor_name()
Copy link
Contributor

Choose a reason for hiding this comment

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

As we call on task instance potentially often and there is a bunch of logic behind this - but w/o DB access - to wire up executor details... should the called get_default_executor_name() method being implement the @cache decorator?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jscheffl, would this return the right executor in a multi executor context?

Copy link
Contributor

Choose a reason for hiding this comment

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

If the executor field is None then the default executor is used during execution - so the logic on API for display is consistent here. If the field on the TaskInstance is filled, then it references to the executor that specifically is wanted.

@kaxil kaxil modified the milestones: Airflow 3.1.2, Airflow 3.1.3 Oct 31, 2025
@dheerajturaga
Copy link
Contributor

@kaxil , I couldn't push to this PR, so I attempted to fix tests in #57706
Feel free to cherry-pick or re-use that PR

@kaxil
Copy link
Member Author

kaxil commented Nov 2, 2025

@kaxil , I couldn't push to this PR, so I attempted to fix tests in #57706

Feel free to cherry-pick or re-use that PR

Cool, would be good if someone can take over since I won't have access to GitHub for next 5-6 weeks

@kaxil kaxil closed this Nov 2, 2025
@dheerajturaga
Copy link
Contributor

@kaxil , I couldn't push to this PR, so I attempted to fix tests in #57706
Feel free to cherry-pick or re-use that PR

Cool, would be good if someone can take over since I won't have access to GitHub for next 5-6 weeks

No-worries, I got it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants