Repository navigation
fix(net): blob route cap, root_path/trailing-slash cap matching, throttled baseline-skip log - #52
Merged
Conversation
…ttled baseline-skip log Follow-ups from the PR #50 review: - Blob uploads get their own route cap, _BLOB_BODY_CAP = _BLOB_CAP + 16 KiB slack, derived from the handler's chunk constant (now defined with the other caps), instead of the 4 MiB ceiling. - _body_cap_for matches the app-relative path: a whole-segment root_path prefix (uvicorn --root-path puts it in scope["path"]) and trailing slashes are stripped first, as Starlette's router does, so /pw/users and /users/ get /users's 64 KiB cap instead of the ceiling. - The "baseline skipped" log line is printed for the first skip and every 100th; the /metrics counter stays exact. - README cap wording fixed (blob uploads are no longer "at the ceiling"). - Tests: exact-cap passes and cap+1 is 413 for /users, /result, /jobs and blobs; GET with a >64 KiB body is 413; trailing-slash and root_path-prefixed paths get the route cap; the log throttle keeps an exact count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Follow-ups from the PR #50 review (
claude-review-pw-50.md, verdict MERGE-WITH-FOLLOWUP). This change touches only the cap table,_body_cap_for, the baseline-skip log, the README cap lines andtests/test_coordinator_limits.py.What changed
POST /jobs/{id}/blobs/{hash}now has its own cap:_BLOB_BODY_CAP = _BLOB_CAP + 16 KiB, which is 528 KiB. It was the 4 MiB ceiling._BLOB_CAPmoved up next to the other caps and is defined only once.application/octet-stream,operator.py:197-202), with no JSON or multipart wrapper. So the extra 16 KiB is only slack: at the boundary,put_blob's own exact 512 KiB check still decides, and it still answerschunk too large._body_cap_for(method, path, root_path)removes a whole-segmentroot_pathprefix before matching, the same way Starlette'sget_route_pathdoes. uvicorn 0.54 buildsscope["path"]asroot_path + path./pw/usersand/users/now get the 64 KiB cap for/usersinstead of falling back to the ceiling.scope["root_path"]through.[baseline] skipped …line is printed for the 1st skip and then every 100th (_BASELINE_SKIP_LOG_EVERY)./metricsbaseline_skipped_busycounter still counts every skip exactly.--root-pathgets the same cap as the plain route./users, on/tasks/{id}/result(4 MiB, spy shows the endpoint ran), on/jobs(4 MiB) and on blobs (512 KiB handler edge plus 528 KiB pre-auth route edge).root_path-prefixed paths get the route cap.Fails on main where behaviour changed
I ran the new test file against
origin/main'scoordinator_app.py: 6 failed, 26 passed. The failing tests:test_blob_route_cap_is_the_chunk_cap_plus_slack_not_the_ceiling. On main it errors on the missing_BLOB_BODY_CAP; without the new route cap, a 528 KiB+1 blob would reach auth and get 401.test_trailing_slash_gets_the_route_cap_not_the_ceiling. On main the request gets a 307 instead of a 413.test_root_path_prefix_gets_the_route_captest_skip_log_is_throttled_but_the_counter_stays_exacttest_busy_slots_skip_the_baseline_with_a_log_line_and_a_countertest_body_cap_for_routes_and_the_ceilingThe exact-cap tests for
/users,/resultand/jobsand the GET test pass on main too. That is expected: they cover behaviour that was already right but untested.Verification (rebased on
003a0a4, headacc1f27)ruff check . && pytest tests/ -q --cov=passiveworkers --cov-fail-under=75→ exit 0:All checks passed!,807 passed, 1 skipped, total coverage 83.64%.python -m py_compile $(git ls-files '*.py')→ exit 0.pyright(now blocking forpassiveworkers/netsince ci: make pyright blocking, pin actions to commit SHAs, add CodeQL #51; installed into the local venv) → exit 0,0 errors, 0 warnings, 0 informations.Not verified
uvicorn --root-pathserver. The root_path test copies uvicorn 0.54'sroot_path + pathscope using Starlette's TestClient (root_path="/pw")./jobsand/resultceilings are unchanged. The review's suggestion of about 256 KiB for/jobswas not part of this brief.🤖 Generated with Claude Code