weiqingy opened a new issue, #701:
URL: https://github.com/apache/flink-agents/issues/701

   ### Search before asking
   
   - [x] I searched in the 
[issues](https://github.com/apache/flink-agents/issues) and found nothing 
similar.
   
   ### Description
   
   
   After #161 lands (PR moves Ollama integration tests into the `it-python` CI 
job which already installs Ollama via `tools/start_ollama_server.sh`), the 
inline Python-3.10-only auto-pull block at the top of the two Ollama test files 
becomes **dead code in every CI cell**:
   
   - 
python/flink_agents/integrations/chat_models/tests/test_ollama_chat_model.py
     (lines 40-47)
   - 
python/flink_agents/integrations/embedding_models/local/tests/test_ollama_embedding_model.py
     (similar block)
   
   The block:
   ```
       try:
           # only auto setup ollama in ci with python 3.10 to reduce ci cost.
           if "3.10" in sys.version:
               subprocess.run(
                   ["bash", f"{current_dir}/start_ollama_server.sh", 
test_model],
                   timeout=300,
                   check=True,
               )
           client = Client()
           ...
   ```
   Why it's dead after #161:
   - ut-python will exclude these files via `-m "not integration"`, so the 
module-import side effect (the subprocess.run) never fires there.
   - it-python matrix runs Python 3.11 and 3.12 only, so the `if "3.10" in 
sys.version` guard is always false. The Ollama install happens at the CI step 
level instead via `bash tools/start_ollama_server.sh`.
   
   ### Proposed cleanup
   
   1. Remove the inline subprocess.run([...start_ollama_server.sh, test_model]) 
block from both Ollama test files.
   2. Keep the `client = Client()` + `skipif(client is None, ...)` portion — it 
still serves as a guard on developer machines without Ollama running.
   3. Confirm with maintainers whether any local-dev workflow depends on the 
inline auto-pull (e.g., is anyone running these tests directly on a Python 3.10 
venv without first starting Ollama?). If yes, document the new prerequisite in 
CONTRIBUTING.md.
   4. Audit similar inline-subprocess patterns in other integration test files 
for the same anti-pattern.
   
   ### Are you willing to submit a PR?
   
   - [x] I'm willing to submit a PR!


-- 
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]

Reply via email to