fix: send Content-Type: application/json when calling inference service#25
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbitRelease Notes
WalkthroughModified the inference request to send a JSON-serializable dictionary via Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ccx_upgrades_data_eng/inference.py`:
- Line 29: The requests.get call assigned to inference_response is misformatted
for Black; reformat the call to requests.get(inference_endpoint,
json=risk_predictors.model_dump(), timeout=5) by breaking arguments onto
separate lines (one argument per line and trailing comma) so Black will accept
it — locate the call using the inference_response variable name and the
requests.get invocation with inference_endpoint and risk_predictors.model_dump()
and update its formatting accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
ccx_upgrades_data_eng/inference.py
79e70b6 to
cdf373e
Compare
Additional Context
Add
Content-Type: application/jsonto upgrades-inference requests, since the header became required by default with move to newer version of FastAPI. This is blocking automatic version bump in RedHatInsights/ccx-upgrades-inference#22Explanation from Claude:
FastAPI 0.133.1 (with newer starlette) is stricter about requiring Content-Type: application/json for JSON body parsingType of change
Testing steps
NA
Checklist
pre-commit run --allpasses