SameerMesiah97 commented on PR #61637: URL: https://github.com/apache/airflow/pull/61637#issuecomment-3940951284
> I am unsure about if it is really a good idea to add a second parameter with additional complexity in logic if there is already one which is (known not be be perfect but already covers 90%). I think adding another second method adds also mental load for users to understand as well as is a risk of inconsistency. > > As there are many users using K8s I am a bit afraid that such change on a "public API" is more costly than the benefits gained. Therefore I was stepping a bit back waiting for other feedback. Still am not really convinced and would ask if other maintainers see a larger benefit - then I'd not block the merge. In a classic vote in devlist I'd give a "0" (not -1 and no +1) The motivation behind this change comes from the discussion in PR #59413, where there was considerable back and forth around which pod phases should be deleted under each enum. While the existing `on_finish_action `parameter is functional and covers most use cases, that thread highlighted how users could prefer different phase groupings for deletion depending on their individual setups. Since Kubernetes defines five pod phases, there are 31 possible non-empty combinations. Encoding specific combinations as additional enums over time could lead to repeated discussions about which groupings should or should not be supported. The intention here is not to replace the existing modes, but to provide a clearly named and optional escape hatch for cases that fall outside the common patterns. This would allow users who require a specific phase combination to configure it explicitly, while keeping the existing enums unchanged and recommended for most scenarios. I do agree that we should limit additional complexity. To mitigate that, I could refine the PR so that `'delete_pods_in_phase'` is an additional enum for `on_finish_action `rather than an override parameter. The new parameter `delete_pods_in_phase` could instead be used to set the phases of the pods to be deleted. As a result of this modification, this PR would not be adding a new parameter but extending the list of options for `on_finish_action`, which I believe would be less disruptive. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
