IEBH-469: Add endpoints for copy to central node functionality#13
IEBH-469: Add endpoints for copy to central node functionality#13vmoshynskyi wants to merge 1 commit intomainfrom
Conversation
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR exposes “copy to central node” functionality via new public FastAPI endpoints, wiring them into the application router and adding test coverage, along with configuration for longer client timeouts.
Changes:
- Added
/v1/central-node/upload(initiate) and/v1/central-node/upload/{upload_key}(wait) endpoints. - Registered the new router and introduced Central Node-specific HTTP client timeouts.
- Added API tests and bumped project version / pre-commit dependency set.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
api/api_central_node.py |
Implements the new Central Node upload init/wait endpoints and proxies requests to DataOps. |
app/api_registry.py |
Registers the Central Node router under /v1. |
config.py |
Adds Central Node-specific timeout settings. |
tests/api/test_api_central_node.py |
Adds tests covering init success/forbidden and wait success. |
pyproject.toml |
Bumps package version to 2.2.79. |
.pre-commit-config.yaml |
Adds a pinned setuptools dependency to the flake8 hook. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| 'operator': self.current_user.username, | ||
| }, | ||
| ) | ||
|
|
||
| return await self.process_response(raw_response) |
There was a problem hiding this comment.
process_response() calls raise_for_status(), which will turn any non-2xx response from DataOps into an UnhandledException (500) instead of forwarding the real status/body (e.g., 4xx from DataOps). For these proxy-style endpoints, return a FastAPI Response using raw_response.content, raw_response.status_code, and filtered headers without raising on status, or override raise_for_response_status() for this class to allow pass-through errors.
| @router.get('/central-node/upload/{upload_key}', summary='Wait file upload authorization from the Central Node.') | ||
| async def wait(self, upload_key: str, authorization: Annotated[str | None, Header()] = None) -> Response: | ||
| if authorization is None: | ||
| raise APIException(error_msg='Missing Authorization header.', status_code=EAPIResponseCode.forbidden.value) | ||
|
|
There was a problem hiding this comment.
The Missing Authorization header check will never run when the header is absent because current_user: Depends(jwt_required) is resolved before the route handler; jwt_required/get_current_identity currently decodes the token unconditionally and will raise, resulting in a 500 instead of the intended 403. Either remove jwt_required for this endpoint (if it should accept a non-JWT auth header), or make the auth dependency handle missing/invalid Authorization cleanly (e.g., raise APIException/401) and then drop this redundant check.
| headers={'Authorization': authorization}, | ||
| ) | ||
|
|
||
| return await self.process_response(raw_response) |
There was a problem hiding this comment.
Same issue as in init: using process_response() will raise on DataOps 4xx/5xx and convert it into a 500 from BFF. For a long-polling/authorization wait endpoint, it’s especially important to pass through DataOps status codes (including non-200) rather than raising.
| return await self.process_response(raw_response) | |
| return Response( | |
| content=raw_response.content, | |
| status_code=raw_response.status_code, | |
| ) |
Expose copy to central node functionality endpoints into public.