potiuk commented on code in PR #62656:
URL: https://github.com/apache/airflow/pull/62656#discussion_r2927314485
##########
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:
**Missing tests.** This is a bug fix that changes serialization behavior,
but no tests were added. Please add tests to
`shared/logging/tests/logging/test_structlog.py` covering:
1. A log event with a non-serializable object (e.g., a custom class
instance) in the event dict doesn't crash and falls back to `str()`.
2. A log event with standard serializable objects still works correctly
(existing `test_json` covers this, but a targeted test for the `safe_default`
path would be valuable).
3. The case where `default` is `None` (if that's possible in practice).
Example:
```python
def test_json_non_serializable_object(structlog_config):
class Unserializable:
def __str__(self):
return "<Unserializable>"
with structlog_config(json_output=True) as bio:
logger = structlog.get_logger("my.logger")
logger.info("Hello", obj=Unserializable())
written = json.load(bio)
assert written["obj"] == "<Unserializable>"
```
--
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]
