Update main with 2.5.0 config values #28076
Conversation
| @@ -2377,7 +2377,7 @@ | |||
| failed worker pods will not be deleted so users can investigate them. | |||
| This only prevents removal of worker pods where the worker itself failed, | |||
| not when the task it ran failed. | |||
| version_added: 1.10.11 | |||
| version_added: 2.5.0 | |||
There was a problem hiding this comment.
I don't think we should change these values?
There was a problem hiding this comment.
(Many others below too. This one is just an example)
There was a problem hiding this comment.
Yeah, this is not nice...section renaming
There was a problem hiding this comment.
Ok. I think this is the correct thing to do. We have moved the configurations to a new section. The new section was created in 2.5.0, therefore, it makes sense that all items in the section are also added in 2.5.0. This was the practice when we moved some items to a new 'database' section. See #22284. WDYT
There was a problem hiding this comment.
I do not think so. This change will leave new-comers an impression that these configurations are really new since 2.5.0, while it's not the truth
There was a problem hiding this comment.
AIRFLOW__KUBERNETES__POD_TEMPLATE_FILE was 1.10.11 while AIRFLOW__KUBERNETES_EXECUTOR__POD_TEMPLATE_FILE started existing in 2.5.0.
Makes me wonder if we should have a new field on the config file, version_renamed or something because, in some way, this was really added in 2.5.0 but practically works down to 1.10.11 with old section where it was originally added in the old section.
cc @ashb
There was a problem hiding this comment.
I have been off for a while... @potiuk what do you think about this? I think the correctness depends on how one looks at it. The section/option is new in 2.5.0 while the old section/option is not new.
If we mark it 1.10.11, users would think it'll work in 1.10.11 but that's not true. It will only work in 1.10.11 with the old section name, not the new one.
There was a problem hiding this comment.
I think it's also misleading now.
We should add a feature IMHO (because as far as I know it does not exist now) to refer to which was deprecated/previous version of the config - and still keep the "from" there. It does not have to be detailed but it should explain that this section has been renamed from [kubernetes] in 2.5.0
There was a problem hiding this comment.
BTW. Glad you are back :). I hope you rested 👍 We missed you 😄
There was a problem hiding this comment.
BTW. Glad you are back :). I hope you rested 👍 We missed you 😄
Thanks! Missed you guys too 😀
ephraimbuddy
requested review from
potiuk,
jedcunningham,
ashb and
dstandish
Because of the change of section, these keys are being updated as if they were added in 2.5.0. These are suggestions from
./dev/validate_version_added_fields_in_config.py