kaxil commented on code in PR #65121:
URL: https://github.com/apache/airflow/pull/65121#discussion_r3081163474
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -2124,6 +2125,7 @@ def supervise(
finally:
if log_path and log_file_descriptor:
log_file_descriptor.close()
+ WRITE_LOCKS.pop(log_file_descriptor)
Review Comment:
`.pop()` without a default will raise `KeyError` if the file descriptor
isn't in the dict. Since this runs in a `finally` block, a `KeyError` here
would shadow whatever exception the `try` block raised.
```suggestion
WRITE_LOCKS.pop(log_file_descriptor, None)
```
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -46,6 +46,7 @@
import psutil
import structlog
from pydantic import BaseModel, TypeAdapter
+from structlog._output import WRITE_LOCKS
Review Comment:
Echoing what Ash and Elad said. One option if you want to ship this before
the upstream PR (hynek/structlog#806) lands: wrap it in try/except so a future
structlog upgrade that moves or removes this internal doesn't take down the
supervisor:
```python
try:
from structlog._output import WRITE_LOCKS
except ImportError:
WRITE_LOCKS = None # type: ignore[assignment]
```
Then guard the pop:
```python
if WRITE_LOCKS is not None:
WRITE_LOCKS.pop(log_file_descriptor, None)
```
--
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]
