wolvery commented on PR #59413: URL: https://github.com/apache/airflow/pull/59413#issuecomment-3719945110
> I quite agree with @SameerMesiah97 here. It's wrong to delete RUNNING and PENDING pods when your flag is "delete_succeeded_pods". When there are some pods running or pending, they might - given enough time turn into either "succeeded" or "failed". That would be a breaking change as well - as explained in the comment from @SameerMesiah97 . > > If you want to implement your change, you should add a new on finish action enum. and when your condition is `!= Failed` - I think the right is "delete_non_failed". > > This is purely a matter of consistency and clarity. When you say that this is unexpected behaviour, then - as somoene who does not know that much about KPO internals - just occasionally look here - it would be absolutely surprising when I chose "delete_succeeded" deleting also PENDING and RUNNING - this is simply not what the name of the action says. > > However, if I would see that I can choose among: > > * delete_pod (default) > * delete_succeeded > * delete_active > * delete_non_failed (new one that you should add) > > Then it's absolultely crystal clear what options I have. > > Our job is not to guess intentions of people and change options to what we think will be more popular and reasonable, but to leave the decision in the hands of the users - presenting them clear and unambiguous options they can choose from - and let them be in control what happens. > > Please add the new option instead of changing semantics of the exsisting one. Ok. I'll work on adding it to the documentation too: We could mention that this option can lead to unsupervised pods in the cluster: pods in a Running or Pending state may be ignored, which can result in tasks being marked as successful even though the option is intended to delete only successful pods. What would be the steps to deprecate delete_succeeded_pods after introducing delete_non_failed_pods? Could I add a recommendation/deprecation warning? -- 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]
