Nataneljpwd commented on code in PR #61637:
URL: https://github.com/apache/airflow/pull/61637#discussion_r2837220085
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -457,6 +466,29 @@ def __init__(
self.container_name_log_prefix_enabled =
container_name_log_prefix_enabled
self.log_formatter = log_formatter
+ @staticmethod
+ def _validate_delete_pods_in_phase(delete_pods_in_phase: Iterable[str] |
str) -> set[str]:
+ if isinstance(delete_pods_in_phase, str):
+ phases = {delete_pods_in_phase}
+ else:
+ phases = set(delete_pods_in_phase)
+
+ valid_phases = {
+ PodPhase.PENDING,
+ PodPhase.RUNNING,
+ PodPhase.FAILED,
+ PodPhase.SUCCEEDED,
+ PodPhase.UNKNOWN,
+ }
+
+ invalid = phases - valid_phases
+ if invalid:
+ raise AirflowException(
+ f"Invalid pod phase(s) in delete_pods_in_phase:
{sorted(invalid)}. Valid phases are: Pending, Running, Failed, Succeeded,
Unknown."
+ )
+
+ return phases
Review Comment:
Should a validation method return the object it validates against? It seems
weird to me to return the exact same object you receive if when invalid we
throw an exception and return nothing
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -1238,11 +1303,11 @@ def process_pod_deletion(self, pod: k8s.V1Pod, *,
reraise=True) -> bool:
)
)
- if should_delete_pod:
- self.log.info("Deleting pod: %s", pod.metadata.name)
- self.pod_manager.delete_pod(pod)
- return True
- self.log.info("Skipping deleting pod: %s", pod.metadata.name)
+ if should_delete_pod:
+ self.log.info("Deleting pod: %s", pod.metadata.name)
+ self.pod_manager.delete_pod(pod)
+ return True
+ self.log.info("Skipping deleting pod: %s", pod.metadata.name)
return False
Review Comment:
Usually when I have a boolean object, I return it instead of returning its
value checked in an if, and avoid returning early in this case, as of now I
can't think of a way for this to be done here, what do you think?
--
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]
