Skip to content

Fix LlamaIndexHook docs to stop claiming Ollama/vLLM support - #72013

Merged
gopidesupavan merged 1 commit into
apache:mainfrom
astronomer:fix-common-ai-llamaindex-scope
Sep 4, 2026
Merged

gopidesupavan merged 1 commit into
apache:mainfrom
astronomer:fix-common-ai-llamaindex-scope

Conversation

@Lee-W

@Lee-W Lee-W commented Aug 23, 2026

Copy link
Copy Markdown
Member

The class docstring, the host field description, and the UI placeholder said host= could point LlamaIndexHook at Ollama or vLLM. get_llm() and get_embedding_model() return LlamaIndex's OpenAI / OpenAIEmbedding classes, which validate model= client-side against LlamaIndex's OpenAI-only model-name allowlists before any request is sent, so an Ollama/vLLM model name is rejected regardless of host. Document the real constraint and why it exists instead of the non-existent Ollama/vLLM support.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: [Tool Name] following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@uros-b

uros-b commented Aug 24, 2026

Copy link
Copy Markdown
Member

+1, LGTM!

@ColtenOuO ColtenOuO left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM overall

Should we also update provider.yaml in this PR?

The connection UI and the provider registry both read from provider.yaml, so the claim this PR is removing is still visible to users.

Two places in the llamaindex connection-types entry still carry it:

  - hook-class-name: airflow.providers.common.ai.hooks.llamaindex.LlamaIndexHook
    hook-name: "LlamaIndex"
    connection-type: llamaindex
    external-services:
      - OpenAI
      - Ollama          # <--
      - vLLM            # <-- 
    ui-field-behaviour:
      ...
      placeholders:
        host: "https://api.openai.com/v1 (optional, for custom endpoints / Ollama)"   # <--

I think we should also remove that and re-generate the get_provider_info.py

WDYT

Comment thread providers/common/ai/docs/hooks/llamaindex.rst Outdated
Comment thread providers/common/ai/src/airflow/providers/common/ai/hooks/llamaindex.py Outdated
@Lee-W
Lee-W force-pushed the fix-common-ai-llamaindex-scope branch from d18390e to 0f61a77 Compare August 25, 2026 12:21
The class docstring, the host field description, and the UI
placeholder said host= could point LlamaIndexHook at Ollama or vLLM.
get_llm() and get_embedding_model() return LlamaIndex's OpenAI /
OpenAIEmbedding classes, which validate model= client-side against
LlamaIndex's OpenAI-only model-name allowlists before any request is
sent, so an Ollama/vLLM model name is rejected regardless of host.
Document the real constraint and why it exists instead of the
non-existent Ollama/vLLM support.
@Lee-W
Lee-W force-pushed the fix-common-ai-llamaindex-scope branch from 0f61a77 to f46ed37 Compare August 27, 2026 11:42
@Lee-W

Lee-W commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

external-services on the llamaindex entry is now OpenAI only, the host placeholder no longer names Ollama, and get_provider_info.py is regenerated.

@ColtenOuO ColtenOuO left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! LGTM

@gopidesupavan
gopidesupavan merged commit c77ec69 into apache:main Sep 4, 2026
157 of 158 checks passed
imrichardwu pushed a commit to imrichardwu/airflow that referenced this pull request Sep 11, 2026
…72013)

The class docstring, the host field description, and the UI
placeholder said host= could point LlamaIndexHook at Ollama or vLLM.
get_llm() and get_embedding_model() return LlamaIndex's OpenAI /
OpenAIEmbedding classes, which validate model= client-side against
LlamaIndex's OpenAI-only model-name allowlists before any request is
sent, so an Ollama/vLLM model name is rejected regardless of host.
Document the real constraint and why it exists instead of the
non-existent Ollama/vLLM support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants