SameerMesiah97 commented on code in PR #68584:
URL: https://github.com/apache/airflow/pull/68584#discussion_r3424432375
##########
airflow-core/tests/unit/jobs/test_triggerer_job.py:
##########
@@ -314,6 +314,31 @@ def fake_run_once(self):
assert events == ["enter", "tick-1", "tick-2", "tick-3", "exit"]
[email protected]("json_logs", [True, False])
+def
test_process_log_messages_configures_logging_matching_json_logs(supervisor_builder,
mocker, json_logs):
+ """``_process_log_messages_from_subprocess`` must reconfigure structlog
with the
+ ``logging.json_logs`` setting, not the ``configure_logging`` default of
``False``.
+
+ Priming this generator calls ``configure_logging()``, which reconfigures
structlog
+ globally. With ``json_logs=True`` but ``json_output`` left to default
``False`` the
+ global logger factory becomes the text ``WriteLogger`` while the
stdout/stderr
+ forwarders emit bytes from the JSON renderer, crashing the triggerer with
+ ``TypeError: can't concat str to bytes`` the first time a trigger
subprocess writes
+ to stdout/stderr. See the regression where enabling json_logs
CrashLoopBackOff-ed
+ triggerers running providers that warn at import time.
Review Comment:
This docstring is too long. I would suggest something like this instead:
```
"""
_process_log_messages_from_subprocess() must reconfigure logging using the
configured ``logging.json_logs`` value rather than the default
``json_output=False``.
This generator reconfigures structlog globally when first primed. Failing to
propagate the configured JSON logging mode can leave the triggerer with an
inconsistent logging configuration and break subprocess log forwarding.
"""
```
--
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]
