[CP] Massive Logs Support to CLI (#347) to v2.16-develop - #359
Conversation
* Fixing CLI logs interrupting for some edge cases
* Using WS timeout, changing log funcs to async and chunked output and cathing connection close errors
* Stop redundant DB commits and unbounded log broadcasts under high log volume
* Fixing and adding to the unit tests
* Fixing Black linting errors
* Wrapping websocket closure with a try/catch block.
* Fix PR review findings: task ordering, UTF-8 decoding, and encoding safety
- TestUIObserver: move __async_updates to instance state (was a shared
class-level list, leaking Task references across runs)
- TestUIObserver: broadcast each flush's chunks in order via one
sequential task instead of one independent task per chunk
- test_harness_client: use explicit UTF-8 encoding for the SDK log file
- test_case: read test_output.txt incrementally with a persistent UTF-8
decoder so split multi-byte characters aren't corrupted across reads
- test_case: stream display_batch_logs()/_log_remaining_content()
instead of loading the whole file into memory
* Fixing ui observer unit test
|
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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 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 |
|
Tick the box to add this pull request to the merge queue (same as
|
|
Gentle reminder: please cherry-pick also the CLI_PR#107 to the CLI repo |
3 participants
Summary
Cherry-picks 35f877f ("[Fix] Massive Logs Support to CLI", originally merged as #347) from
v2.15.1-developontov2.16-develop, which never received it.Why
While investigating a report that test executions "run fast but get stuck at the last step, seemingly processing a huge number of log lines," I traced it to four compounding issues in the log pipeline that are exactly what #347 already fixed upstream:
test_db_observer.py): every ~0.5s log-flush tick re-queued the sametest_run_executionobject (with its ever-growing.loglist) onto a plainQueue. At the end of the run, draining that queue ran a synchronoussession.commit()on the event loop once per queued item, redundantly re-saving the same, increasingly large log blob over and over — this is the "stuck at the end" symptom.test_ui_observer.py): a burst of log lines was sent as one multi-MB websocket message with no yield point during JSON serialization, which can stall the event loop long enough to miss keepalive pongs.main.py): uvicorn's default 20sws_ping_timeoutwasn't overridden, so the stall from Updating notices, code of conduct, and readme #2 could get the websocket dropped mid-broadcast.test_case.py): the incremental log tailer had encoding/step-tracking issues that could reprocess large chunks oftest_output.txtrepeatedly.v2.16-developis missing this fix entirely (confirmed viagit merge-base --is-ancestor 35f877freturning false), whilev2.16-developis what recent feature work (e.g. #357) is based on — so this regression is currently live on that line.Conflict resolution
One cherry-pick conflict in
test_collections/matter/sdk_tests/support/tests/python_tests/test_python_test_case.py: both branches independently appended unrelated test functions at the same insertion point. Resolved by keeping both sides' additions in full — verified via AST comparison that every function from both parent commits is present in the merged file with an identical AST to its original (i.e. no test was dropped, corrupted, or duplicated), then ranblackto normalize the seam's blank-line spacing.Testing
black,isort,flake8,mypyall pass on every changed file (mypy's 1 remaining finding —test_db_observer.py'sinsp.sessionattr-defined error — is pre-existing onv2.16-developprior to this cherry-pick, confirmed by running mypy against the file before applying this PR).v2.16-developsince diverging).pytestsuite (Docker/DB-dependent) was not run in the authoring environment; please run./scripts/test-local.shbefore merge.