Nataneljpwd commented on code in PR #61110:
URL: https://github.com/apache/airflow/pull/61110#discussion_r2732962615
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/spark_kubernetes.py:
##########
@@ -279,6 +283,9 @@ def find_spark_job(self, context, exclude_checked: bool =
True):
self.log.info("`try_number` of pod: %s",
pod.metadata.labels.get("try_number", "unknown"))
return pod
+ def _get_field_selector(self) -> str:
+ return
f"status.phase!={PodPhase.RUNNING},status.phase!={PodPhase.PENDING}"
+
Review Comment:
> I believe this will exclude pending and running pods. Are you sure this
was what you wanted? Maybe you wanted to select running or pending pods but I
am not sure field selectors support OR conditions.
>
> I think it would better not to alter the query but filter afterwards
instead. Unless, there is something I am missing here.
The field supports and only, meaning that select pods where not (status is
pending and status is running) which translates to select pods where status is
pending or running, according to De Morgan's laws
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/spark_kubernetes.py:
##########
@@ -279,6 +283,9 @@ def find_spark_job(self, context, exclude_checked: bool =
True):
self.log.info("`try_number` of pod: %s",
pod.metadata.labels.get("try_number", "unknown"))
return pod
+ def _get_field_selector(self) -> str:
+ return
f"status.phase!={PodPhase.RUNNING},status.phase!={PodPhase.PENDING}"
+
Review Comment:
> I believe this will exclude pending and running pods. Are you sure this
was what you wanted? Maybe you wanted to select running or pending pods but I
am not sure field selectors support OR conditions.
>
> I think it would better not to alter the query but filter afterwards
instead. Unless, there is something I am missing here.
The field supports and only, meaning that select pods where not (status is
pending and status is running) which translates to select pods where status is
pending or running, according to De Morgan's laws
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/spark_kubernetes.py:
##########
@@ -279,6 +283,9 @@ def find_spark_job(self, context, exclude_checked: bool =
True):
self.log.info("`try_number` of pod: %s",
pod.metadata.labels.get("try_number", "unknown"))
return pod
+ def _get_field_selector(self) -> str:
+ return
f"status.phase!={PodPhase.RUNNING},status.phase!={PodPhase.PENDING}"
+
Review Comment:
> I believe this will exclude pending and running pods. Are you sure this
was what you wanted? Maybe you wanted to select running or pending pods but I
am not sure field selectors support OR conditions.
>
> I think it would better not to alter the query but filter afterwards
instead. Unless, there is something I am missing here.
The field supports and only, meaning that select pods where not (status is
pending and status is running) which translates to select pods where status is
pending or running, according to De Morgan's laws
--
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]
