XD-DENG commented on code in PR #27611:
URL: https://github.com/apache/airflow/pull/27611#discussion_r1038790399
##########
airflow/executors/kubernetes_executor.py:
##########
@@ -256,7 +256,10 @@ def __init__(
def run_pod_async(self, pod: k8s.V1Pod, **kwargs):
"""Runs POD asynchronously"""
- pod_mutation_hook(pod)
+ try:
+ pod_mutation_hook(pod)
+ except Exception as e:
+ raise PodMutationHookException(e)
Review Comment:
Hi @hterik , thanks for the feedback!
I tested the two different ways below. I find the difference is minor, and I
personally prefer the current way where the detailed exception reason appears
in the same line as the `PodMutationHookException`, so it's clearer to the log
reader (this is also the original reason for what I wrote it this way). WDYT?
## Case -1
```python
from airflow.exceptions import PodMutationHookException
try:
int("a")
except Exception as e:
raise PodMutationHookException(e)
```
We get
```
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
ValueError: invalid literal for int() with base 10: 'a'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 4, in <module>
airflow.exceptions.PodMutationHookException: invalid literal for int() with
base 10: 'a'
```
## Case -2
If we use the way you suggested
```python
from airflow.exceptions import PodMutationHookException
try:
int("a")
except Exception as e:
raise PodMutationHookException() from e
```
We get
```
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
ValueError: invalid literal for int() with base 10: 'a'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "<stdin>", line 4, in <module>
airflow.exceptions.PodMutationHookException
```
--
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]
