You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2026/09/16 15:11:03 UTC

Re: [PR] Add hardcoded model to the gen ai tuning system test [airflow]

potiuk commented on PR #67355:
URL: https://github.com/apache/airflow/pull/67355#issuecomment-5699779623

   Thanks for this, and apologies it sat for so long. Your diagnosis was correct, and the fix that landed is essentially the one this PR was heading towards — so I want to close it with the reasoning rather than let it keep aging.
   
   **What changed.** #72323 ("Fix models selection in genai tests", merged 2026-09-08) removed `_get_actual_model` from this system test entirely and replaced it with a small `llm_models.py` helper next to it:
   
   ```python
   SFT_ENABLED_GEMINI_MODEL = "gemini-3.5-flash"
   
   def get_sft_enabled_gemini_model() -> str:
       return _get_model_env_variable("SFT_ENABLED_GEMINI_MODEL", SFT_ENABLED_GEMINI_MODEL)
   ```
   
   The test now just does `SOURCE_MODEL = llm_models.get_sft_enabled_gemini_model()`. That is the same conclusion you reached — stop discovering the model at runtime and pin one that actually supports tuning — with the pin overridable through an environment variable, which is what my earlier comment asked for. It also addresses the point about `use_hardcoded_model=True` leaving the discovery path as dead code, by deleting that path outright.
   
   **Where that leaves this PR.** The function it patches no longer exists on `main`, so the branch now conflicts (`mergeStateStatus: DIRTY`) and there is nothing left for the change to attach to. The specific pin here, `gemini-2.5-flash-lite`, has also been overtaken — `main` is on `gemini-3.5-flash` for the SFT-enabled slot.
   
   **One correction to the record.** Earlier in this thread it was suggested this could be closed because the issue was mitigated in #68665. That turned out not to be right at the time: #68665 was itself closed without being merged, so nothing had actually landed then. The fix only arrived with #72323 a few weeks ago. Flagging that so the history reads accurately for anyone who finds this thread later.
   
   Closing as superseded. Thank you for spotting the problem and for the patience — the system test is in better shape because it was raised.
   
   ---
   Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
   


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org