versionchooser: move from connexion/aiohttp to fastapi #3435
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR fully migrates the Version Chooser service from connexion/aiohttp to FastAPI, replaces all aiohttp response types with FastAPI responses, removes legacy frontend assets and static handlers, updates the Vue upload logic, adapts tests for the new response interfaces, refactors Docker login models to Pydantic, and updates project dependencies accordingly. Class diagram for DockerLoginInfo refactor to Pydantic modelclassDiagram
class DockerLoginInfo {
+bool root = True
+str username = ""
+str password = ""
+str registry = DEFAULT_DOCKER_REGISTRY
}
DockerLoginInfo <|-- pydantic.BaseModel
Class diagram for new CommandLineArgs dataclassclassDiagram
class CommandLineArgs {
+bool debug
+str host
+int port
+from_args() CommandLineArgs
}
Class diagram for new FastAPI routers and VersionChooser usageclassDiagram
class VersionChooser {
+get_version()
+set_version(image, tag)
+pull_version(repository, tag)
+delete_version(image, tag)
+get_available_local_versions()
+get_available_versions(repository)
+load(data)
+restart()
+get_bootstrap_version()
+set_bootstrap_version(tag)
}
class version_router_v1
class bootstrap_router_v1
class docker_router_v1
class index_router_v1
version_router_v1 --> VersionChooser : Depends
bootstrap_router_v1 --> VersionChooser : Depends
docker_router_v1 --> DockerLoginInfo
docker_router_v1 --> get_docker_accounts
docker_router_v1 --> make_docker_login
docker_router_v1 --> make_docker_logout
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @nicoschmdt - I've reviewed your changes - here's some feedback:
- Consider refactoring the repeated JSONResponse error and success constructions in VersionChooser into a small helper to reduce duplication and centralize response formatting.
- The StreamingResponse media_type is set to "application/x-www-form-urlencoded"—you may want to switch to a JSON or newline-delimited JSON content type to better reflect the payload.
- The routers create a new aiodocker.Docker client and VersionChooser per request—verify this request-scoped lifecycle meets your performance and resource-cleanup requirements.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider refactoring the repeated JSONResponse error and success constructions in VersionChooser into a small helper to reduce duplication and centralize response formatting.
- The StreamingResponse media_type is set to "application/x-www-form-urlencoded"—you may want to switch to a JSON or newline-delimited JSON content type to better reflect the payload.
- The routers create a new aiodocker.Docker client and VersionChooser per request—verify this request-scoped lifecycle meets your performance and resource-cleanup requirements.
## Individual Comments
### Comment 1
<location> `core/services/versionchooser/api/v1/routers/version.py:71` </location>
<code_context>
+
+
+@version_router_v1.post("/load", summary="Load a docker tar file")
+async def load(file: UploadFile = File(...), version_chooser: VersionChooser = Depends(get_docker_client)) -> Any:
+ data = await file.read()
+ return await version_chooser.load(data)
</code_context>
<issue_to_address>
The load endpoint reads the entire file into memory, which may cause issues with large uploads.
Consider processing the uploaded file in chunks or streaming it to the Docker client to avoid high memory usage and improve scalability.
</issue_to_address>
### Comment 2
<location> `core/services/versionchooser/api/v1/routers/docker.py:22` </location>
<code_context>
+)
+
+
+@docker_router_v1.post("/login", summary="Login Docker daemon to a registry")
+async def docker_login(request: DockerLoginInfo) -> None:
+ return make_docker_login(request)
</code_context>
<issue_to_address>
The docker_login endpoint is defined as async but returns a synchronous function.
Consider making 'make_docker_login' async or running it in a thread pool to maintain consistency and avoid potential blocking issues.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
@docker_router_v1.post("/login", summary="Login Docker daemon to a registry")
async def docker_login(request: DockerLoginInfo) -> None:
return make_docker_login(request)
=======
import asyncio
@docker_router_v1.post("/login", summary="Login Docker daemon to a registry")
async def docker_login(request: DockerLoginInfo) -> None:
return await asyncio.to_thread(make_docker_login, request)
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `core/services/versionchooser/api/v1/routers/docker.py:27` </location>
<code_context>
+ return make_docker_login(request)
+
+
+@docker_router_v1.post("/logout", summary="Logout Docker daemon from a registry")
+async def docker_logout(request: DockerLoginInfo) -> Any:
+ return make_docker_logout(request)
</code_context>
<issue_to_address>
The docker_logout endpoint is async but returns a synchronous function.
Consider making 'make_docker_logout' async or running it in a thread pool to prevent blocking the event loop if it performs I/O.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
@docker_router_v1.post("/logout", summary="Logout Docker daemon from a registry")
async def docker_logout(request: DockerLoginInfo) -> Any:
return make_docker_logout(request)
=======
import asyncio
@docker_router_v1.post("/logout", summary="Logout Docker daemon from a registry")
async def docker_logout(request: DockerLoginInfo) -> Any:
return await asyncio.to_thread(make_docker_logout, request)
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Williangalvani
approved these changes
Jul 28, 2025
joaomariolago
approved these changes
Jul 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix #1866
Move versionchooser endpoints to FastApi;
Remove legacy frontend folder from
core/services/versionchooser;Update frontend upload logic for FastAPI backend compatibility.
To test use the following docker image: nicoschmdt/blueos-core:aiohttp-to-fastapi
Summary by Sourcery
Migrate VersionChooser from connexion/aiohttp to FastAPI, replacing legacy code and updating handlers, dependencies, frontend logic, and tests for FastAPI compatibility
New Features:
Bug Fixes:
Enhancements:
Build:
Tests:
Chores: