Fix: Improve HTTP API structure and async handler usage (#569)#2063
Fix: Improve HTTP API structure and async handler usage (#569)#2063thchann wants to merge 1 commit intoroboflow:mainfrom
Conversation
|
Thanks for contribution - since we are really busy this week, postponing review |
|
Review is ongoing - we've identified couple of issues, seem like @dkosowski87 has some feedback here |
dkosowski87
left a comment
There was a problem hiding this comment.
Nice clean up, thanks 🚀 left some comments, after fixing those we will be able to merge this.
|
|
||
| from fastapi import APIRouter, HTTPException, Query | ||
|
|
||
| rom inference.core.version import __version__ |
There was a problem hiding this comment.
| rom inference.core.version import __version__ | |
| from inference.core.version import __version__ |
| from inference.core.interfaces.http.error_handlers import with_route_exceptions | ||
| from inference.core.interfaces.http.orjson_utils import orjson_response | ||
| from inference.core.managers.base import ModelManager | ||
| from inference.core.utils.model_alias import resolve_roboflow_model_alias |
There was a problem hiding this comment.
This was moved inference/models/aliases.py
|
|
||
| @router.post( | ||
| "/infer/semantic_segmentation", | ||
| response_model=Union[InstanceSegmentationInferenceResponse, StubResponse], |
There was a problem hiding this comment.
| response_model=Union[InstanceSegmentationInferenceResponse, StubResponse], | |
| response_model=Union[SemanticSegmentationInferenceResponse, StubResponse], |
| if DEPTH_ESTIMATION_ENABLED: | ||
|
|
||
| @router.post( | ||
| "/infer/depth-estimation", |
There was a problem hiding this comment.
Probably we could set up a different route as this one will be shadowed by the one in inference.
| router = APIRouter() | ||
|
|
||
| def process_workflow_inference_request( | ||
| workflow_request, |
| countinference=countinference, | ||
| service_secret=service_secret, | ||
| ) | ||
| app.include_router(create_inference_router(model_manager=self.model_manager)) | ||
|
|
||
| if LMM_ENABLED or MOONDREAM2_ENABLED: |
There was a problem hiding this comment.
This part should also go the inference module. It is nested under if not LAMBDA and not GCP_SERVERLESS:
| max_concurrent_steps=WORKFLOWS_MAX_CONCURRENT_STEPS, | ||
| prevent_local_images_loading=True, | ||
| ) | ||
| return WorkflowValidationStatus(status="ok") | ||
|
|
||
| if WEBRTC_WORKER_ENABLED: |
There was a problem hiding this comment.
We could probably extract this one also.
| """Health endpoint for Kubernetes liveness probe.""" | ||
| return {"status": "healthy"} | ||
|
|
||
| return router No newline at end of file |
There was a problem hiding this comment.
Always add a new line at the end of the file
| ), | ||
| countinference: Optional[bool] = None, | ||
| service_secret: Optional[str] = None, | ||
| ): |
There was a problem hiding this comment.
Maybe this is a result on working on a previous vesion of the repo. But this si missing:
if not SAM3_FINE_TUNED_MODELS_ENABLED:
if not inference_request.model_id.startswith("sam3/"):
raise HTTPException(
status_code=501,
detail="Fine-tuned SAM3 models are not supported on this deployment. Please use a workflow or self-host the server.",
)
Let's update the branch and see if it will be brought back.
| ): | ||
| """ | ||
| Runs the YOLO-World zero-shot object detection model. | ||
| @app.get( |
There was a problem hiding this comment.
Notebook could also go to a separate module
What does this PR do?
HttpInterfaceinto modular FastAPI routers underinference/core/interfaces/http/routes/(inference, models, workflows, stream, core_models, legacy, info, health).with_route_exceptions, while only truly async code (e.g. stream manager, WebRTC worker) remainsasync+with_route_exceptions_async.Type of Change
Testing
I have tested this change locally
I have added/updated tests for this change
Ran: pytest -m "not slow" tests/
Test details:
tests/inference/unit_tests/core/interfaces/http/test_remote_processing_time_middleware.pyimports the refactoredhttp_apiand passes.Checklist
Additional Context