Backcompat for non inheriting executors

[dstandish]

Some executors don't inherit from BaseExecutor. 2.10 change assumes they do.

This fixes that.

Resolves #41891

[dstandish]

sorry -- had wrong base; fixed now.

[potiuk]

I think we have the rule that we generally implement such fixes in main and backport them to v2-*test - this is the first exception so far. Not sure if justified.

[potiuk]

cc: @utkarsharma2 @ephraimbuddy @kaxil @eladkal -> WDYT? ^^

[dstandish]

I think we have the rule that we generally implement such fixes in main and backport them to v2-*test - this is the first exception so far. Not sure if justified.

I can't speak to that necessarily but this is the type of backcompat thing that I'd want to chop in 3.0 anyway. If you think it's better, we could add to main, then remove it with changelog. But it's such a small thing!

[kaxil]

My answer: theorethically it does, if somoene uses currently released cncf.kubernetes provider and CeleryKubernetesExecutor or LocalKubernetesExecutor in Airflow 3.

If it affects "main" currently, then yeah I think we should fix and merge in main too. And then cherry-pick / separate PR (this can be that PR and you can create one for main too)

[dstandish]

If it affects "main" currently, then yeah I think we should fix and merge in main too. And then cherry-pick / separate PR (this can be that PR and you can create one for main too)

I guess jarek is saying that "theoretically" it affects main but practicall speaking we will drop support for hybrid executors / executors-that-don't-inherit-from-base-executor in 3.0 and / or somehow indicate that providers are not assumed to be future compat with 3.0.

anyway, no strong feeling on it. just ultimately do want this backcompat in 3.0 -- somehow or another.

[potiuk]

anyway, no strong feeling on it. just ultimately do want this backcompat in 3.0 -- somehow or another.

Yep. then I think we should add it to main and back-port even if it is "such a small change" - I guess in this case "let's do all things the same way" trumps "but this one is so small and likely to be not needed".

[potiuk]

I think individual's convenience in this case is less important that "overall consistency" - and we have to pay the overhead price for it. We already do that by having to run Pull requests for all backports rather than individual cherry-picks as it was in previous v2-* which means that the committers authoring/merging changes have more work to do at the expense of less work by the release manager. I guess that's the sacrifice we all should make now.

And I am not sarcastic - I rigorously follow those rules, it's just the price to pay and we should all be aware why we are paying it and we should share the burden.

[kaxil]

I think individual's convenience in this case is less important that "overall consistency" - and we have to pay the overhead price for it. We already do that by having to run Pull requests for all backports rather than individual cherry-picks as it was in previous v2-* which means that the committers authoring/merging changes have more work to do at the expense of less work by the release manager. I guess that's the sacrifice we all should make now.

Agreed. At the point of the PR being open, if it affects main too, we should ideally fix it in main even if a future change will remove it.

[potiuk]

Would be great to get that one for 2.10.1 @ephraimbuddy @utkarsharma2

[potiuk]

(Still - this one needs main + backport as discussed before).

[utkarsharma2]

Would be great to get that one for 2.10.1 @ephraimbuddy @utkarsharma2

Sure @potiuk I'll include this for 2.10.1

[potiuk]

Something strange happend when building images -> closing/reopening to rebuild