feat: make BaseService methods async#474
Merged
olivermeyer merged 8 commits intomainfrom Mar 20, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR converts the BaseService contract to an async interface (health() / info()), propagates that change across service implementations and key call sites (CLI/GUI), and adds a new FastAPI DI helper (BaseService.get_service()) plus a settings accessor.
Changes:
- Make
BaseService.health()/BaseService.info()async and update all service implementations accordingly. - Update system CLI/GUI and platform tests to await or run async health/info calls.
- Add
BaseService.get_service()(FastAPI dependency factory),BaseService.settings(), and accompanying tests/docs.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/aignostics/utils/service_test.py | Adds tests for BaseService.get_service() caching/behavior and settings() accessor. |
| tests/aignostics/platform/service_test.py | Updates platform service health test to async def + await. |
| src/aignostics/utils/_service.py | Makes base API async; adds get_service() and settings(). |
| src/aignostics/utils/CLAUDE.md | Updates docs/examples for async services and DI helper. |
| src/aignostics/platform/_service.py | Converts platform service health/info to async signature. |
| src/aignostics/system/_service.py | Converts system service health_static/health/info to async and awaits discovered services. |
| src/aignostics/system/_gui.py | Switches GUI health/info loading to await the async service methods. |
| src/aignostics/system/_cli.py | Runs async health/info via asyncio.run(...) in CLI commands. |
| src/aignostics/application/_service.py | Converts application service health/info to async signature. |
| src/aignostics/application/_cli.py | Runs async system health check via asyncio.run(...) before submitting runs. |
| src/aignostics/application/_gui/_page_application_describe.py | Updates GUI to call async system health (currently via asyncio.run). |
| src/aignostics/bucket/_service.py | Converts bucket service health/info to async signature. |
| src/aignostics/dataset/_service.py | Converts dataset service health/info to async signature. |
| src/aignostics/notebook/_service.py | Converts notebook service health/info to async signature. |
| src/aignostics/qupath/_service.py | Converts QuPath service health/info to async signature. |
| src/aignostics/wsi/_service.py | Converts WSI service health/info to async signature. |
| src/aignostics/CLAUDE.md | Updates root docs’ service examples to async signatures. |
Codecov Report❌ Patch coverage is
|
olivermeyer
force-pushed
the
feat/bridge-baseservice
branch
from
b8c68d2 to
67cf115
Compare
olivermeyer
force-pushed
the
feat/bridge-baseservice
branch
from
67cf115 to
b17bd91
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK’s service abstraction (BaseService) to use async health() / info() methods, and propagates that change through service implementations and key call sites (system aggregation, CLI, and GUI). It also adds a FastAPI dependency helper (get_service()) and a public settings accessor.
Changes:
- Make
BaseService.health()/BaseService.info()async and update allBaseServicesubclasses accordingly. - Add
BaseService.get_service()(cached FastAPI dependency factory) andBaseService.settings()accessor. - Update system CLI/GUI and selected GUI pages to call async health/info, and add unit tests for the new BaseService helpers.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/aignostics/utils/service_test.py | Adds unit coverage for BaseService.get_service() caching/DI behavior and settings(). |
| tests/aignostics/platform/service_test.py | Updates platform service health test to await the new async API. |
| src/aignostics/utils/_service.py | Converts abstract methods to async; adds get_service() DI helper and settings() accessor. |
| src/aignostics/utils/CLAUDE.md | Updates utils module docs/examples for async service methods and new helpers. |
| src/aignostics/CLAUDE.md | Updates top-level docs/examples for async service methods. |
| src/aignostics/application/_service.py | Converts service health() / info() to async. |
| src/aignostics/bucket/_service.py | Converts service health() / info() to async. |
| src/aignostics/dataset/_service.py | Converts service health() / info() to async. |
| src/aignostics/notebook/_service.py | Converts service health() / info() to async. |
| src/aignostics/platform/_service.py | Converts service health() / info() to async. |
| src/aignostics/qupath/_service.py | Converts service health() / info() to async. |
| src/aignostics/wsi/_service.py | Converts service health() / info() to async. |
| src/aignostics/system/_service.py | Makes system aggregation APIs async and awaits discovered services’ health/info. |
| src/aignostics/system/_cli.py | Bridges async health/info via asyncio.run() for Typer CLI commands. |
| src/aignostics/system/_gui.py | Updates system GUI page to await async health/info. |
| src/aignostics/gui/_frame.py | Updates launchpad health polling to await async system health. |
| src/aignostics/application/_cli.py | Bridges async system health check via asyncio.run() in CLI. |
| src/aignostics/application/_gui/_page_application_describe.py | Updates application describe page to await async system health. |
Collaborator
Author
|
One reply to all Copilot comments above: making all calls async is out of scope here, I just want to unify the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
olivermeyer
requested a review
from helmut-hoffer-von-ankershoffen
as a code owner
olivermeyer
added
claude
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR aligns the SDK’s BaseService interface with Bridge by making health()/info() async, adding a FastAPI-friendly dependency factory (get_service) plus a settings accessor (settings), and updating service callers across CLI/GUI to accommodate the async contract.
Changes:
- Make
BaseService.health()andBaseService.info()asyncand update concrete service implementations accordingly. - Add
BaseService.get_service()(cached FastAPI dependency factory) andBaseService.settings()accessor, plus new unit tests. - Update CLI/GUI call sites to use
await/asyncio.run(...)for async health/info retrieval.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/aignostics/utils/service_test.py | Adds unit tests for get_service() caching/behavior and settings(), plus coroutine assertions for health/info. |
| tests/aignostics/platform/service_test.py | Updates the platform service health test to await the async health(). |
| src/aignostics/wsi/_service.py | Converts health()/info() to async to match BaseService. |
| src/aignostics/utils/_service.py | Introduces get_service() and settings(), and makes health()/info() async abstract methods. |
| src/aignostics/utils/CLAUDE.md | Updates service examples/docs to reflect async methods and new DI/settings helpers. |
| src/aignostics/system/_service.py | Makes health_static, health, and info async and updates internal aggregation calls to await. |
| src/aignostics/system/_gui.py | Updates GUI to await async health/info calls (removes previous run.cpu_bound usage). |
| src/aignostics/system/_cli.py | Wraps async service calls in asyncio.run() for CLI execution. |
| src/aignostics/qupath/_service.py | Converts health()/info() to async. |
| src/aignostics/platform/_service.py | Converts health()/info() to async. |
| src/aignostics/notebook/_service.py | Converts health()/info() to async. |
| src/aignostics/gui/_frame.py | Updates periodic health polling to await async SystemService.health_static(). |
| src/aignostics/dataset/_service.py | Converts health()/info() to async. |
| src/aignostics/bucket/_service.py | Converts health()/info() to async. |
| src/aignostics/application/_service.py | Converts health()/info() to async. |
| src/aignostics/application/_gui/_page_application_describe.py | Updates GUI flow to await system health check once during page load. |
| src/aignostics/application/_cli.py | Uses asyncio.run(SystemService.health_static()) in the sync CLI path. |
| src/aignostics/CLAUDE.md | Updates top-level docs to reflect async service method signatures. |
Comments suppressed due to low confidence (1)
src/aignostics/system/_gui.py:91
await Service.info(...)does blocking system calls (psutil.cpu_percent(interval=...), etc.) insideService.info. Running it directly on the NiceGUI event loop can stall the UI during the measurement interval. Consider running info collection in a worker thread (nicegui.run.io_bound/cpu_bound) or refactor the implementation to avoid blocking calls in the async path.
editor.set_visibility(False)
spinner.set_visibility(True)
mask_secrets_switch.set_visibility(False)
info = await Service.info(include_environ=True, mask_secrets=mask_secrets)
if info is None:
properties["content"] = {"json": "Info retrieval failed."} # type: ignore[unreachable]
else:
properties["content"] = {"json": info}
ff75bfc to
3613113
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK’s service abstraction to align BaseService with Bridge by adding FastAPI-friendly dependency injection helpers and converting the core health()/info() contract to async, then propagating those signature changes through services, CLI, GUI, and tests.
Changes:
- Added
BaseService.get_service()(FastAPI dependency factory) andBaseService.settings()accessor. - Made
BaseService.health()/BaseService.info()async and updated concrete service implementations + affected call sites (CLI/GUI/system aggregation). - Added/updated unit tests to cover the new service helpers and async behavior.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/aignostics/utils/service_test.py | New tests for BaseService.get_service() caching semantics, settings(), and async method shape. |
| tests/aignostics/platform/service_test.py | Updates a platform health test to async def and awaits Service.health(). |
| src/aignostics/utils/_service.py | Introduces get_service() + settings() and makes abstract health/info async. |
| src/aignostics/utils/CLAUDE.md | Updates documentation examples for async health/info and documents get_service() + settings(). |
| src/aignostics/wsi/_service.py | Updates service to async health/info to satisfy new BaseService contract. |
| src/aignostics/platform/_service.py | Updates platform service to async health/info. |
| src/aignostics/bucket/_service.py | Updates bucket service to async health/info. |
| src/aignostics/dataset/_service.py | Updates dataset service to async health/info. |
| src/aignostics/application/_service.py | Updates application service to async health/info. |
| src/aignostics/qupath/_service.py | Updates qupath service to async health/info. |
| src/aignostics/notebook/_service.py | Updates notebook service to async health/info. |
| src/aignostics/system/_service.py | Updates system aggregation/service methods (health_static, health, info) to async and awaits discovered services. |
| src/aignostics/system/_cli.py | Wraps async service calls via asyncio.run() in CLI commands. |
| src/aignostics/application/_cli.py | Wraps SystemService.health_static() via asyncio.run() for CLI preflight health check. |
| src/aignostics/system/_gui.py | Updates GUI to await async health_static() / info() directly. |
| src/aignostics/gui/_frame.py | Updates periodic health UI to await SystemService.health_static() directly. |
| src/aignostics/application/_gui/_page_application_describe.py | Updates application describe page to await SystemService.health_static() and uses the result in stepper logic. |
| src/aignostics/CLAUDE.md | Updates top-level docs examples to use async health/info. |
olivermeyer
force-pushed
the
feat/bridge-baseservice
branch
from
3613113 to
8279335
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK’s service abstraction to better align with Bridge by making BaseService.health() / BaseService.info() asynchronous, adding a FastAPI DI helper (get_service) plus a settings() accessor, and updating CLI/GUI/tests to use the new async contract.
Changes:
- Make
BaseService.health()andBaseService.info()async, and update all concrete service implementations accordingly. - Add
BaseService.get_service()(cached FastAPI dependency factory) andBaseService.settings()accessor + unit tests. - Update CLI/GUI/test call sites to
awaitservice calls or wrap them viaasyncio.run(); migrate platform/system health checks fromurllib3tohttpxasync client.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/aignostics/utils/service_test.py | Adds unit tests for get_service() caching, settings(), and async coroutine behavior. |
| tests/aignostics/system/service_pooling_test.py | Removes pooling tests corresponding to removal of urllib3 pool usage. |
| tests/aignostics/system/cli_test.py | Updates mocks to AsyncMock for async health(). |
| tests/aignostics/platform/service_test.py | Converts tests to async and updates mocking for httpx.AsyncClient. |
| tests/aignostics/application/gui_test.py | Runs blocking CliRunner.invoke via asyncio.to_thread in async GUI tests. |
| src/aignostics/utils/_service.py | Adds get_service() + settings() and makes health()/info() async abstract methods. |
| src/aignostics/utils/CLAUDE.md | Updates docs/examples for async service methods + DI usage. |
| src/aignostics/system/_service.py | Makes health/info async, switches network calls to httpx, and avoids blocking CPU sampling by using asyncio.sleep. |
| src/aignostics/system/_gui.py | Updates GUI to await async health_static() / info(). |
| src/aignostics/system/_cli.py | Wraps async service calls with asyncio.run() for synchronous Typer commands. |
| src/aignostics/platform/_service.py | Migrates health checks from urllib3 to async httpx and makes service methods async. |
| src/aignostics/application/_service.py | Makes health() / info() async. |
| src/aignostics/application/_gui/_page_application_describe.py | Updates system health usage to async/await. |
| src/aignostics/application/_cli.py | Wraps async system health check with asyncio.run(). |
| src/aignostics/bucket/_service.py | Makes health() / info() async. |
| src/aignostics/dataset/_service.py | Makes health() / info() async. |
| src/aignostics/wsi/_service.py | Makes health() / info() async. |
| src/aignostics/qupath/_service.py | Makes health() / info() async. |
| src/aignostics/notebook/_service.py | Makes health() / info() async. |
| src/aignostics/gui/_frame.py | Updates periodic health check to await async SystemService.health_static(). |
| src/aignostics/CLAUDE.md | Updates top-level docs to reflect async service API. |
olivermeyer
force-pushed
the
feat/bridge-baseservice
branch
from
f1d0f72 to
dcc8028
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK’s service abstraction to support async health() / info() methods (to align with Bridge), adds FastAPI-friendly service injection via BaseService.get_service(), and updates affected CLI/GUI codepaths plus tests accordingly.
Changes:
- Make
BaseService.health()/BaseService.info()async and update all concrete service implementations. - Add
BaseService.get_service()(FastAPI dependency factory) andBaseService.settings()accessor. - Update CLI/GUI callers and tests for async behavior; migrate some health/info networking to
httpx.AsyncClient.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/aignostics/utils/service_test.py | Adds unit tests for BaseService.get_service() caching and settings() accessor; verifies coroutine nature of health/info. |
| tests/aignostics/system/service_test.py | Adds unit tests for new CPU frequency helper behavior. |
| tests/aignostics/system/service_pooling_test.py | Removes tests tied to the deleted urllib3 pool behavior. |
| tests/aignostics/system/cli_test.py | Updates CLI tests to mock async health() using AsyncMock. |
| tests/aignostics/platform/service_test.py | Converts platform service health tests to async and updates mocking to httpx.AsyncClient. |
| tests/aignostics/application/gui_test.py | Runs CliRunner.invoke in a thread to avoid event-loop conflicts with asyncio.run in CLI commands. |
| tests/aignostics/application/cli_test.py | Adjusts a run submission test to include --force. |
| src/aignostics/utils/_service.py | Introduces get_service() + settings() and makes health/info abstract methods async. |
| src/aignostics/utils/CLAUDE.md | Updates docs for async service methods and documents FastAPI DI + settings accessor. |
| src/aignostics/system/_service.py | Switches system health/info to async and migrates network calls to httpx; adds CPU frequency helper. |
| src/aignostics/system/_gui.py | Updates GUI to await async system health/info methods. |
| src/aignostics/system/_cli.py | Wraps async system service calls with asyncio.run() for sync Typer commands. |
| src/aignostics/platform/_service.py | Migrates platform health checks from urllib3 to httpx and makes health async. |
| src/aignostics/application/_cli.py | Wraps SystemService.health_static() with asyncio.run() for sync CLI preflight. |
| src/aignostics/application/_service.py | Makes health/info async for application service. |
| src/aignostics/application/_gui/_page_application_describe.py | Awaits system health in application describe flow; updates force-option logic accordingly. |
| src/aignostics/gui/_frame.py | Updates launchpad health indicator to await async system health. |
| src/aignostics/bucket/_service.py | Makes health/info async for bucket service. |
| src/aignostics/dataset/_service.py | Makes health/info async for dataset service. |
| src/aignostics/wsi/_service.py | Makes health/info async for WSI service. |
| src/aignostics/qupath/_service.py | Makes health/info async for QuPath service. |
| src/aignostics/notebook/_service.py | Makes health/info async for notebook service. |
| src/aignostics/CLAUDE.md | Updates top-level docs to reflect async health/info. |
Comments suppressed due to low confidence (2)
src/aignostics/system/_gui.py:56
Service.health_static()is typed to always returnHealth, so theif health is None:branch is now dead code after switching fromrun.cpu_bound(...)to a direct await. Consider removing theNonecheck (and thetype: ignore), or wrap the await intry/exceptand sethealth=Noneexplicitly on failure if you want to keep the fallback UI.
health = await Service.health_static()
if health is None:
properties["content"] = {"json": "Health check failed."} # type: ignore[unreachable]
else:
properties["content"] = {"json": health.model_dump()}
src/aignostics/system/_gui.py:91
Service.info(...)now returns adict(non-optional), soif info is None:is unreachable in normal execution. Consider removing theNonebranch, or catch exceptions around the await and display the fallback message only on error.
info = await Service.info(include_environ=True, mask_secrets=mask_secrets)
if info is None:
properties["content"] = {"json": "Info retrieval failed."} # type: ignore[unreachable]
else:
properties["content"] = {"json": info}
arne-aignx
reviewed
Mar 20, 2026
Contributor
arne-aignx
left a comment
arne-aignx
left a comment
There was a problem hiding this comment.
If I correctly grasped the scope here
the MR focuses on aligning the BaseService implementation with the one in bridge.
This requires us to make some methods async.
Good switch to httpx
There are some minor refactorings sprinkled into the MR. 👍
With the switch from urllib3 to httpx the test setup can be improved by using pytest-httpx to define the mock queries. but thats beyond the scope
One minor question added.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… checks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract CPU frequency measurement into a dedicated private static method to reduce cognitive complexity of Service.info() from 17 to 10. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace 9 inline occurrences of the patch target string "aignostics.platform._service.httpx.AsyncClient" with a single module-level constant _PATCH_HTTPX_ASYNC_CLIENT, following the existing _PATCH_AUTH_GETTER convention. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
olivermeyer
force-pushed
the
feat/bridge-baseservice
branch
from
c4e9bd5 to
a3aeec0
Compare
olivermeyer
force-pushed
the
feat/bridge-baseservice
branch
from
a3aeec0 to
47031f2
Compare
|
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
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.
Unifying the
BaseServicewith Bridge.The main changes are:
get_servicemethod for FastAPI injection and asettingsmethodhealthandinfomethods async + adjusting concrete servicesasyncio.run()healthandinfoI am not making all methods on the various service implementations async.