Repository navigation
fix: log only replies that were sent, keep the drain line after a second signal, skip logging work when off - #105
Merged
Conversation
…ond signal, skip logging work when off
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #104.
Three defects found by review after the logging and owner-cap merges, none of which leaks anything:
finally, so a client reset mid-reply still logged a 200, and the busy 503 was logged even when its non-blocking send raised. A reply is now recorded as sent only after its body is written (thesend_erroroverride does the same after the standard library's whole reply, including a headers-only HEAD refusal), and the busy 503 is logged only when the send delivered every byte; otherwise no request line, as for a connection that ends before any reply. The protocol guide's Logging section says so.server_closewaited one second for the drain thread. The thread now publishes its fields and sets an Event before callingshutdown(), andserver_closewrites the line from that result if the thread has not, exactly once and beforestop, waiting up to five seconds; past that,stopproceeds without the line, which the guide also says.BRIDGE_LOG=offthe request timer, the route and method helpers, the job timings and the drain's snapshot still ran; a cheaplog.enabled()check now skips them.Tests, seen failing first where the behaviour was wrong: a reply whose first write fails writes no request line, nor does a failed or short busy send; a drain thread held past one second still writes exactly one
drainline withinterrupted: truebeforestop, and past the five-second wait writes none; with logging off the timer and field construction are skipped, not just the output; captured lines for a 429 and a 503 draining with status, owner, schema and no sentinel; and the owner cap with cancelled jobs (concurrency 1, cap 2: cancelled jobs keep the cap until one expires, while another owner still submits).Files:
worker/ai_bridge/server.py,worker/ai_bridge/jobs.py,worker/ai_bridge/log.py,tests/python/test_server.py,tests/python/test_jobs.py,tests/python/test_log.py,docs/protocol.md,docs/testing.md,CHANGELOG.md.Checks: the compose suites could not run locally because Docker Desktop stopped responding on the development machine mid-run ("Docker Desktop is unable to start"); 51 targeted tests ran green in the containers before that. The whole Python suite was then run directly on the Windows host with Python 3.13: 415 tests, with the same fifteen platform-only failures as main on that host (file-mode and shell-script cases that the containers cover) and one order-dependent error in
test_a_busy_503_is_logged_without_anything_read, which passes when run alone. The CI gate on this pull request is therefore the check of record for this head; it runs the full sequence including ruff and mypy at strict. Review: independent review by Codex (gpt-6.1-sol): no defect, every listed test present and distinguishing main's behaviour.