dstandish commented on a change in pull request #19359:
URL: https://github.com/apache/airflow/pull/19359#discussion_r741641790



##########
File path: airflow/executors/kubernetes_executor.py
##########
@@ -599,13 +599,14 @@ def sync(self) -> None:
                 try:
                     self.kube_scheduler.run_next(task)
                 except ApiException as e:
-                    if e.reason == "BadRequest":
-                        self.log.error("Request was invalid. Failing task")
+                    if e.reason in ("BadRequest", "Unprocessable Entity"):
+                        self.log.error(f"Pod creation failed with reason 
{e.reason!r}. Failing task")

Review comment:
       yeah i can see that point.  another reason i went with `reason` was ... 
well... the code was already using `reason` for `BadRequest`.  And you know it 
occurred to me the possibility that the status codes have less fidelity than 
the reasons, i.e. possibly less precise, so that if we were to change this to 
status code, perhaps we would catch more scenarios in this block than was 
originally intended. e.g if 400 can be `BadRequest` and `LessBadRequest`, we 
might start catching something we weren't supposed to.
   
   but, i've seen no evidence of this, and since the majority seems to be in 
favor of status code, i'll make the change.




-- 
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]


Reply via email to