Skip to content

Commit 7c3b12d

Browse files
committed
Address review feedback
- Update the stale `_MAX_SEGMENT_BODY_BYTES` comment. It still claimed the cap was unenforced for chunked uploads, which stopped being true in d70a7a4 when `_body_iter` started counting cumulative bytes. Documented both enforcement points (Content-Length up front, streamed bytes as the real backstop). - Document that a concurrency slot is deliberately held for the whole proxied call — launch plus inference — and why that differs from the conversion and Whitebox caps, which enqueue a background job and return immediately. - Annotate the shared `calls` buffers on `_FakeHttpx` / `_ErrorHttpx` as `ClassVar[list]` so the intentional class-level sharing is explicit. - Add the missing `-> Iterator[bytes]` return annotation to `_oversized_body`.
1 parent c728b7f commit 7c3b12d

2 files changed

Lines changed: 16 additions & 7 deletions

File tree

backend/geolibre_server/geolibre_server/app/ml.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,21 @@
6969

7070
# Cap concurrent segmentation proxy requests so a burst of large uploads cannot
7171
# exhaust sidecar memory or starve the event loop. Mirrors the conversion
72-
# engine's MAX_IN_FLIGHT_JOBS pattern.
72+
# engine's MAX_IN_FLIGHT_JOBS pattern, with one difference worth knowing: those
73+
# endpoints enqueue a background job and return immediately, whereas this is a
74+
# streaming proxy, so a slot is held for the whole call — the samgeo-api launch
75+
# (_HEALTH_TIMEOUT_SECS) plus inference (_PROXY_TIMEOUT_SECS). That is
76+
# deliberate: the resource being rationed is the held connection and the
77+
# in-flight upload, not a queue position.
7378
MAX_IN_FLIGHT_SEGMENT_REQUESTS = 4
7479
_segment_lock = threading.Lock()
7580
_segment_in_flight = 0
7681

77-
# Reject uploads whose Content-Length exceeds this (100 MiB). When the header
78-
# is missing (chunked transfer) the limit is not enforced — the backend's own
79-
# size handling applies.
82+
# Reject uploads larger than this (100 MiB). Enforced twice: once up front on
83+
# Content-Length so an oversized upload is refused before any bytes are read,
84+
# and again on the cumulative bytes seen in ``_body_iter``, which is what
85+
# actually covers a client that omits the header (chunked transfer) or sends a
86+
# non-numeric one.
8087
_MAX_SEGMENT_BODY_BYTES = 100 * 1024 * 1024
8188

8289
# Guards the launch-or-reuse decision for the child process.

backend/geolibre_server/tests/test_ml.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import asyncio
1111
import json
1212
import subprocess
13+
from collections.abc import Iterator
14+
from typing import ClassVar
1315

1416
import pytest
1517

@@ -62,7 +64,7 @@ class _FakeHTTPError(Exception):
6264
class _FakeHttpx:
6365
"""Minimal stand-in for the httpx module used by ml.py."""
6466

65-
calls: list = []
67+
calls: ClassVar[list] = []
6668
HTTPError = _FakeHTTPError
6769
AsyncClient = _FakeAsyncClient
6870

@@ -290,7 +292,7 @@ def test_segment_releases_slot_after_upstream_failure(monkeypatch):
290292
from geolibre_server.app.main import app
291293

292294
class _ErrorHttpx:
293-
calls: list = []
295+
calls: ClassVar[list] = []
294296
HTTPError = Exception
295297

296298
class AsyncClient:
@@ -358,7 +360,7 @@ def test_segment_rejects_oversized_chunked_stream(monkeypatch):
358360
monkeypatch.setattr(ml, "_require_httpx", lambda: _FakeHttpx)
359361
monkeypatch.setattr(ml, "_ensure_server", lambda: "http://backend:9")
360362

361-
def _oversized_body():
363+
def _oversized_body() -> Iterator[bytes]:
362364
yield b"x" * 50
363365

364366
client = TestClient(app)

0 commit comments

Comments
 (0)