[python] Remove Python-3.10-only Ollama auto-pull block in integration tests#831
Conversation
…on tests After apache#161 moved the Ollama integration tests into the it-python CI job (which installs Ollama via tools/start_ollama_server.sh at the step level), the inline 'if "3.10" in sys.version: subprocess.run(...)' block is dead in every CI cell: ut-python excludes these via -m "not integration", and it-python runs Python 3.11/3.12 so the guard is always false. Remove the block and the now-unused subprocess/sys/Path imports and current_dir from both Ollama test files, and delete the two orphaned test-dir start_ollama_server.sh copies that only the inline block referenced. The Client() + skipif guard is kept so the tests still skip cleanly on machines without Ollama.
github-actions
Bot
added
doc-not-needed
| ["bash", f"{current_dir}/start_ollama_server.sh", test_model], | ||
| timeout=300, | ||
| check=True, | ||
| ) |
There was a problem hiding this comment.
The script start_ollama_server.sh installs the Ollama server and pulls the required models. After moving the integration tests into the it-python pipeline, we no longer need to install the Ollama server separately; however, we still need to pull the test models being used. We can use the pull_model method in test_utils.py.
There was a problem hiding this comment.
Good catch -tools/start_ollama_server.sh only installs the server; it doesn't pull models, so after dropping the inline block these tests would skip on a missing model in CI rather than actually run.
Switched both files to client = pull_model(test_model) from e2e_tests/test_utils.py — the same helper the e2e integration tests already use. It pulls the model and returns the client (or None), so the existing skipif(client is None) guard is unchanged. Also dropped the now-unused from ollama import Client.
Verified locally: the embedding test now runs instead of skipping. Fixed in 3b8d9e1.
| if "3.10" in sys.version: | ||
| subprocess.run( | ||
| ["bash", f"{current_dir}/start_ollama_server.sh"], timeout=300, check=True | ||
| ) |
There was a problem hiding this comment.
Done here too — same pull_model(test_model) switch as the chat test. 3b8d9e1
The canonical tools/start_ollama_server.sh only installs the Ollama server; it does not pull models. After removing the inline auto-pull block, the two integration tests no longer pulled their test models (qwen3:1.7b, all-minilm:22m), so in CI they would skip on a missing model instead of running. Replace the inline Client()/model_found guard in both Ollama integration tests with pull_model(test_model) from e2e_tests/test_utils -- the shared helper already used by every e2e integration test. It pulls the model and returns the client (or None), preserving the exact skipif(client is None) semantics while restoring the model pull.
2 participants
Linked issue: #701
Purpose of change
Follow-up cleanup to #161. That PR moved the Ollama integration tests into the
it-pythonCI job, which installs Ollama at the step level viatools/start_ollama_server.sh(ci.yml:192/229/275). That made the inlineif "3.10" in sys.version: subprocess.run(...)auto-pull block at the top of the two Ollama test files dead code in every CI cell:ut-pythonexcludes these tests via-m "not integration", so the module-import side effect never fires there.it-pythonruns Python 3.11 and 3.12 only, so theif "3.10" in sys.versionguard is always false.This PR:
subprocess,sys, andPathimports and thecurrent_dirvariable (each was used only inside the removed block).client = Client()+@pytest.mark.skipif(client is None, ...)guard, so the tests still skip cleanly on developer machines without Ollama running.start_ollama_server.shcopies under the test directories (integrations/chat_models/tests/andintegrations/embedding_models/local/tests/). After removing the inline block, nothing referenced them; CI uses the canonicaltools/start_ollama_server.sh.Audit of issue item 4:
grep -rln "subprocess.run" python/flink_agents/integrations/returns only these two files — no other integration test carries the same anti-pattern.One open question for maintainers (issue item 3): does any local-dev workflow rely on the inline auto-pull — i.e. is anyone running these tests directly on a Python 3.10 venv without first starting Ollama? If so, the new prerequisite (start Ollama first, e.g.
bash tools/start_ollama_server.sh) is worth a line in CONTRIBUTING.md. Theskipifguard already turns a missing Ollama into a clean skip rather than a failure, so this PR does not add a doc note; happy to follow up if preferred.Tests
No new tests — this removes dead code. Verified locally:
ruff checkandruff format --checkpass on both files.pytest -m integrationon both files: chat tests pass (Ollama running locally), embedding test skips cleanly — confirming the removed imports/symbols leave no broken references and theskipifguard still works.API
No. Test-only change, no public API touched.
Documentation
doc-not-needed