feat: make BaseService methods async#474
Draft
olivermeyer wants to merge 9 commits intofeat/health-degradedfrom
Draft
feat: make BaseService methods async#474olivermeyer wants to merge 9 commits intofeat/health-degradedfrom
olivermeyer wants to merge 9 commits intofeat/health-degradedfrom
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ervices [intermediate] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s [intermediate] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ntermediate] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…)/info() [intermediate] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. |
Comment on lines
305
to
307
| # Check system health and determine if force option should be available | ||
| system_healthy = bool(SystemService.health_static()) | ||
| system_healthy = bool(asyncio.run(SystemService.health_static())) | ||
| unhealthy_tooltip = None |
Move SystemService.health_static() call from the sync inner function _add_application_version_selection_section() to the outer async _page_application_describe(), capturing the result via closure. Also fix gui/_frame.py to await health_static() directly instead of passing it to run.cpu_bound(), which ran the async function in a subprocess with no event loop, causing "coroutine was never awaited". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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.
No description provided.