Skip to content

Conversation

@o-nikolas
Copy link
Contributor

The run task instance capability was removed from the Airflow UI and API in #29706 but the base executor interface and subclasses which support it was not updated.

Screenshot from 2023-08-14 15-11-14

supports_ad_hoc_ti_run was only used in the run task logic so it can now be removed.


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

The run task instance capability was removed from the Airflow UI and API
in apache#29706 but the base executor interface and subclasses which support
it was not updated.
@boring-cyborg boring-cyborg bot added area:providers area:Scheduler including HA (high availability) scheduler provider:celery provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels Aug 14, 2023
@o-nikolas
Copy link
Contributor Author

CC @bbovenzi @jedcunningham

Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

-0 from me for this change.
(I won't get in the way, but I'd rather we didn't do this)

:param parallelism: how many jobs should run at one time. Set to ``0`` for infinity.
"""

supports_ad_hoc_ti_run: bool = False
Copy link
Member

Choose a reason for hiding this comment

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

BaseExecutor is a part of the public interface of Airflow, is it safe to remove this config?

We don't use this config in Airflow, but we don't know if the users use it in custom plugins or custom executors.

Copy link
Member

Choose a reason for hiding this comment

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

I tend to agree; maybe a deprecation would be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Run task api/UI feature was certainly public and we ripped that out without too much worry.
I don't think there will be any impact for removing this relatively obscure field that was not used elsewhere. What we uphold to the public is that this interface, when implemented, will plug into Airflow. Worrying about removing this particular field is falling prey to XKCD 1172 IMHO.

@o-nikolas o-nikolas closed this Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers area:Scheduler including HA (high availability) scheduler provider:celery provider:cncf-kubernetes Kubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants