Repository navigation
CLI: fix stream_sink re-entrancy, log yield interval and trailing log drop - #131
Conversation
logger.add(stream_sink, ...) was missing the enqueue=True its own comment claimed, so it ran synchronously on whichever thread logged the message - including the SSE-server thread logging its own send progress in logs_http_server.py, re-entering the same sink/queue from inside itself. Adding enqueue=True moves it to loguru's dedicated worker thread, closing that path. Since stream_sink now runs asynchronously, stop_log_streaming() calls logger.complete() before stopping the handler, so queued log lines aren't dropped by add_log_entry() turning into a no-op (or the client set being torn down) while they're still in flight to the worker. Lowered LOG_RECORD_YIELD_INTERVAL from 200 to 50. The backend sends up to LOG_RECORDS_BROADCAST_CHUNK_SIZE=200 records per websocket message (test_ui_observer.py), so the old value only yielded once per message; the lower value makes a single large message yield multiple times instead of running uninterrupted end to end. Part of project-chip/certification-tool#1119
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
What
logger.add(stream_sink, ...)inth_cli/test_run/logging.pynow passes theenqueue=Trueits own comment claimed, so it runs on loguru's worker thread and no longer re-enters itself from the SSE-server thread (logs_http_server.pylogs its own send progress).stop_log_streaming()callslogger.complete()before stopping the handler, so queued lines are not dropped.LOG_RECORD_YIELD_INTERVAL200 → 50: the backend sends up to 200 records per websocket message, so 200 only yielded once per message.Why
Items from the original report #1119. This is a correctness/robustness fix; no separate speed measurement. For context, running with
--no-streaming(no log viewer) cut CLI CPU from ~20 s to ~5 s per run without changing wall time, so the CLI is not on the critical path of these runs.Tests
No test was added for the threading change itself. The unit tests below were not run locally before opening this PR (they need the Docker/Postgres stack); CI will run them. Behaviour was also checked on a real device: TC-ACE-1.3 through the CLI on an Ubuntu test host.
Closes #1172
Part of #1119