Skip to content

feat: Add health_check() method to OllamaAdapter#57

Merged
hidai25 merged 1 commit intohidai25:mainfrom
gauravxthakur:health-check-ollama-adapter
Mar 5, 2026
Merged

feat: Add health_check() method to OllamaAdapter#57
hidai25 merged 1 commit intohidai25:mainfrom
gauravxthakur:health-check-ollama-adapter

Conversation

@gauravxthakur
Copy link
Contributor

Description

Implemented the async health_check() method for OllamaAdapter to verify server availability.

Related Issue

#36
Fixes #36

Type of Change

  • New feature (non-breaking change which adds functionality)

Changes Made

  • Added health_check() to OllamaAdapter using the /api/tags endpoint.
  • Used httpx.RequestError to handle connection failures, following the existing HTTPAdapter pattern.
  • Added 3 unit tests in tests/test_adapters.py covering: server success (200), server failure (500), and unreachable host (wrong port).

Testing

Test Configuration

  • Python Version: 3.13.7
  • OS: Windows 11

Tests Added/Modified

  • Unit tests added/updated
  • All tests pass locally
  • Test coverage maintained or improved

Manual Testing Steps

  1. Run py -m pytest tests/test_adapters.py -v to verify all 33 adapter tests pass.
  2. Run py -m mypy evalview/adapters/ollama_adapter.py to ensure type safety.

Checklist

Code Quality

  • My code follows the project's style guidelines
  • I have run mypy and fixed any type errors
  • I have performed a self-review of my own code

Documentation

  • I have added docstrings to new functions/classes

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Dependencies

  • I have checked that no new dependencies were added unnecessarily

Screenshots

evalview2

Additional Notes

The issue tips suggested using aiohttp. However, after reviewing the current implementation of HTTPAdapter.health_check() and pyproject.toml, I found the project is standardized on httpx. I used httpx to maintain consistency with the existing codebase.

Reviewer Notes

Please verify that the transition to httpx for this adapter aligns with the project's long-term direction, as it differs from the suggestion in the issue description.


By submitting this pull request, I confirm that:

  • I have read and followed the CONTRIBUTING.md guidelines
  • I agree to the project's CODE_OF_CONDUCT.md
  • My contribution is my own work and I have the right to contribute it

@hidai25 hidai25 merged commit 87f6e7a into hidai25:main Mar 5, 2026
8 checks passed
@hidai25
Copy link
Owner

hidai25 commented Mar 5, 2026

Hey @gauravxthakur great contribution!

The implementation is clean and correct — /api/tags is the right endpoint,
httpx.RequestError correctly catches both connection errors and timeouts, and the httpx
decision over aiohttp was exactly right. Good instinct to check the existing codebase
rather than follow the issue suggestion blindly.

Two tiny nitpicks (not blocking):

  • test_health_check_failure could add assert_called_once_with like the success test
    does, just for consistency
  • A test for httpx.TimeoutException would be nice since timeouts are a common Ollama
    failure mode

Neither is a blocker and merging as-is. Thanks for the thorough PR description and for
running mypy. Looking forward to more contributions!

@gauravxthakur
Copy link
Contributor Author

Thanks @hidai25 for the feedback and for merging!

Really glad the httpx decision aligned with the project's direction. Noted on the assert_called_once_with and the TimeoutException test. I'll make sure to include those patterns in my next PR.

muhammadrashid4587 pushed a commit to muhammadrashid4587/eval-view that referenced this pull request Mar 11, 2026
…ge (hidai25#59)

## Summary

- Split `[all]` optional dependency extra to exclude
`sentence-transformers` (and its ~2GB torch/CUDA transitive deps)
- New `[all-local]` extra for users who want local GPU embeddings
- Docker image uses `[all]` by default — stays lightweight (~200MB vs
~2.5GB)
- Updated design doc and README to reflect the split

Closes hidai25#57

## Design Conformance

| # | Requirement | Source | Status | Evidence |
|---|---|---|---|---|
| 1 | `[all]` extra does NOT include `sentence-transformers` | Issue
hidai25#57, AC 1 | CONFORMANT | `pyproject.toml:27` |
| 2 | New `[all-local]` extra includes everything | Issue hidai25#57, AC 2 |
CONFORMANT | `pyproject.toml:28` |
| 3 | Dockerfile uses `--extra all` (no change needed) | Issue hidai25#57, AC 3
| CONFORMANT | `Dockerfile:17,22` |
| 4 | `uv.lock` regenerated after the split | Issue hidai25#57, AC 4 |
CONFORMANT | uv.lock diff |
| 5 | README documents the extras split | Issue hidai25#57, AC 6 | CONFORMANT |
`README.md:31-32,45-46` |
| 6 | Design doc updated with `[all-local]` | Design doc, lines 773-778
| CONFORMANT | `docs/design.md:778` |

Reviewed by @architect-reviewer — all requirements CONFORMANT.

## Test plan

- [ ] `uv sync --extra all` does NOT install sentence-transformers
- [ ] `uv sync --extra all-local` DOES install sentence-transformers
- [ ] `docker build -t test .` produces a slim image (~200MB)
- [ ] All 227 existing tests pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add health_check() method to OllamaAdapter

2 participants