potiuk commented on code in PR #62656:
URL: https://github.com/apache/airflow/pull/62656#discussion_r2927322785
##########
shared/logging/src/airflow_shared/logging/structlog.py:
##########
@@ -316,7 +316,17 @@ def json_dumps(msg, default):
"event": msg.pop("event"),
**msg,
}
- return msgspec.json.encode(msg, enc_hook=default)
+
+ def safe_default(obj):
+ """Fall back to str() if the default enc_hook fails or is
None."""
+ if default is not None:
Review Comment:
**Minor performance:** `safe_default` is a new closure allocated on every
call to `json_dumps` (i.e., every JSON log line). Consider hoisting it out of
the function body:
```python
def _make_safe_default(default):
def safe_default(obj):
if default is not None:
try:
return default(obj)
except TypeError:
pass
return str(obj)
return safe_default
def json_dumps(msg, default):
msg = {
"timestamp": msg.pop("timestamp"),
"level": msg.pop("level"),
"event": msg.pop("event"),
**msg,
}
return msgspec.json.encode(msg, enc_hook=_make_safe_default(default))
```
Or, if `default` is always the same callable, cache it once. That said, this
is a minor concern — closure creation is cheap in Python. Up to you whether to
address it.
--
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]
