fix: two-layer gzip decompression for all inbound routes - #160
Conversation
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
8227c5c to
4639c6e
Compare
|
PR image build and manifest generation completed successfully! 📦 PR image: 🗂️ CI manifests |
4639c6e to
9e39299
Compare
| @@ -478,13 +480,11 @@ async def consume_cloud_event( | |||
| ) | |||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail=msg) | |||
| logger.info("KServe Inference Input %s received.", payload.id) | |||
| # if a match is found, the payload is auto-deleted from data | |||
There was a problem hiding this comment.
are these comments not a useful thing to keep?
| @@ -532,8 +531,6 @@ async def consume_cloud_event( | |||
| "message": f"Output payload {payload.id} processed successfully", | |||
| } | |||
|
|
|||
| # Defensive programming: this should never happen due to type annotation | |||
| # but adding explicit fallback for type safety | |||
| @@ -516,7 +516,6 @@ async def consume_cloud_event( | |||
| ) | |||
| if partial_input is not None: | |||
| if not isinstance(partial_input, KServeInferenceRequest): | |||
| # This should never happen - indicates storage interface error | |||
b59e0af to
63b4728
Compare
63b4728 to
6afc88e
Compare
Addresses review feedback from @RobGeada on PR #160. Three comments were removed during the gzip middleware refactor that document non-obvious behavior: - get_partial_payload() auto-deletes the matched partial on retrieval - Type guard checks are defensive (should never happen at runtime) - Final raise is unreachable due to type annotation (explicit fallback) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Sudip Sinha <Sudip.Sinha@RedHat.com>
Summary
Fixes gzip decompression for KServe inference payloads across all three inbound paths:
/data/upload(FastAPI)/consumer/kserve/v2(FastAPI)POST /) (FastAPI)Mirrors the two-layer fix from Java TrustyAI service: trustyai-explainability/trustyai-explainability#707
Problem
TrustyAI fails to parse gzip-compressed inference payloads from KServe logger sidecar with error:
The gzip magic bytes
0x1F 0x8Bare being parsed as JSON. The initial gzip middleware was scoped only to/data/upload, leaving two paths uncovered:/consumer/kserve/v2— FastAPI endpoint but outside middleware scopePOST /) — Knative Eventing bypasses standard HTTP middlewareDesign Philosophy
HTTP
Content-Encodingis a transport-layer concern, not an application-layer concern. Handling it per-endpoint creates coverage gaps — every new endpoint becomes vulnerable by default until someone remembers to add decompression.The correct approach is layered defense:
Transport layer first: Enable middleware-level decompression to handle all HTTP requests before they reach endpoint handlers
Application layer fallback: When a framework bypasses the transport layer (Knative Eventing strips
Content-Encodingheader but leaves body gzipped), add endpoint-specific decompression as a safety netThis ensures:
Solution
Two-layer fix:
Middleware layer (FastAPI endpoints):
GzipRequestMiddleware.DEFAULT_PATHSfrom("/data/upload",)to("*",)Application layer (CloudEvent endpoints):
decompress_if_gzip()utility insrc/endpoints/consumer/gzip_utils.py0x1F 0x8B)Content-Encodingheader is missingChanges
src/middleware/gzip_middleware.pyDEFAULT_PATHSto("*",)src/endpoints/consumer/gzip_utils.pysrc/endpoints/consumer/consumer_endpoint.pysrc/endpoints/data/data_upload.pytests/middleware/test_gzip_middleware_unit.pytests/endpoints/consumer/test_cloud_event_gzip.pyNet: +245 / -24 lines
Test Plan
uv run pytest tests/middleware/test_gzip_middleware_unit.py -v- All tests passuv run pytest tests/endpoints/consumer/test_cloud_event_gzip.py -v- All tests pass/data/upload,/consumer/kserve/v2,/)Content-EncodingheaderRelated
🤖 Generated with Claude Code