weiqingy opened a new pull request, #831: URL: https://github.com/apache/flink-agents/pull/831
Linked issue: #701 ### Purpose of change Follow-up cleanup to #161. That PR moved the Ollama integration tests into the `it-python` CI job, which installs Ollama at the step level via `tools/start_ollama_server.sh` (ci.yml:192/229/275). That made the inline `if "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-python` excludes these tests via `-m "not integration"`, so the module-import side effect never fires there. - `it-python` runs Python 3.11 and 3.12 only, so the `if "3.10" in sys.version` guard is always false. This PR: - Removes the dead auto-pull block from both Ollama test files, along with the now-unused `subprocess`, `sys`, and `Path` imports and the `current_dir` variable (each was used only inside the removed block). - Keeps the `client = Client()` + `@pytest.mark.skipif(client is None, ...)` guard, so the tests still skip cleanly on developer machines without Ollama running. - Deletes the two orphaned `start_ollama_server.sh` copies under the test directories (`integrations/chat_models/tests/` and `integrations/embedding_models/local/tests/`). After removing the inline block, nothing referenced them; CI uses the canonical `tools/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. The `skipif` guard 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 check` and `ruff format --check` pass on both files. - `pytest -m integration` on both files: chat tests pass (Ollama running locally), embedding test skips cleanly — confirming the removed imports/symbols leave no broken references and the `skipif` guard still works. ### API No. Test-only change, no public API touched. ### Documentation - [x] `doc-not-needed` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
