Feat: Add Ollama Cloud API support#10389
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request adds API key support to the Ollama component with header-based authentication, introduces a default API URL constant, expands test coverage for authentication and URL handling, and updates a dependency version in the News Aggregator starter project. Changes
Sequence DiagramsequenceDiagram
participant User
participant Component as ChatOllamaComponent
participant API as Ollama API
rect rgb(200, 220, 255)
Note over User,API: Cloud URL with API Key
User->>Component: set base_url=DEFAULT_OLLAMA_API_URL
Component->>Component: update_build_config() triggers
Component->>Component: api_key becomes required
end
rect rgb(220, 240, 220)
Note over Component,API: Model Discovery with Auth
Component->>Component: build headers with Bearer token
Component->>API: GET /tags (with headers)
API-->>Component: model list
Component->>API: POST /show (with headers for each model)
API-->>Component: model capabilities
end
rect rgb(240, 220, 220)
Note over User,API: Local URL (no auth)
User->>Component: set base_url=localhost:11434
Component->>Component: api_key set to advanced/optional
Component->>Component: headers property returns None
Component->>API: GET /tags (without headers)
API-->>Component: model list
end
rect rgb(200, 220, 255)
Note over Component,API: Build Model with Client Config
Component->>Component: headers present → include client_kwargs
Component->>Component: build llm_params with client_kwargs
User->>Component: create ChatOllama instance
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes involve introducing a new authentication feature across multiple interconnected files (constants, main component, and extensive test coverage), requiring careful verification of header propagation logic, conditional API key behavior based on URL type, and test assertions reflecting the updated HTTP client usage patterns. While individual pieces are straightforward, the coordination between authentication logic, URL handling, and field visibility toggling requires thorough understanding of the complete flow. Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lfx/src/lfx/components/ollama/ollama.py (1)
270-287: Fix build_config refresh: use the edited URL, normalize, and avoid brittle equalityTwo issues:
- Uses exact equality against DEFAULT_OLLAMA_API_URL; fails with trailing slashes or “/v1”.
- Validates/fetches models with self.base_url even when base_url is the field being edited.
Proposed patch:
@@ - if field_name in {"model_name", "base_url", "tool_model_enabled", "api_key"}: - if field_name == "base_url": - # API key is required for cloud Ollama - if field_value != DEFAULT_OLLAMA_API_URL: - build_config["api_key"]["advanced"] = True - build_config["api_key"]["required"] = False - build_config["api_key"]["value"] = None - else: - build_config["api_key"]["advanced"] = False - build_config["api_key"]["required"] = True - if await self.is_valid_ollama_url(self.base_url): + if field_name in {"model_name", "base_url", "tool_model_enabled", "api_key"}: + # Prefer the candidate URL under edit; normalize for comparison/requests + candidate_url = field_value if field_name == "base_url" else self.base_url + candidate_url = (candidate_url or "").rstrip("/").removesuffix("/v1") + cloud_base = DEFAULT_OLLAMA_API_URL.rstrip("/") + if field_name == "base_url": + # API key is required for cloud Ollama + if candidate_url == cloud_base: + build_config["api_key"]["advanced"] = False + build_config["api_key"]["required"] = True + else: + build_config["api_key"]["advanced"] = True + build_config["api_key"]["required"] = False + build_config["api_key"]["value"] = None + if candidate_url and await self.is_valid_ollama_url(candidate_url): tool_model_enabled = build_config["tool_model_enabled"].get("value", False) or self.tool_model_enabled - build_config["model_name"]["options"] = await self.get_models( - self.base_url, tool_model_enabled=tool_model_enabled - ) + build_config["model_name"]["options"] = await self.get_models( + candidate_url, tool_model_enabled=tool_model_enabled + ) else: build_config["model_name"]["options"] = []
🧹 Nitpick comments (3)
src/lfx/src/lfx/components/ollama/ollama.py (3)
242-247: Headers passed for URL validation — OKUsing self.headers here is fine; local instances with no key will pass None. Consider adding a request timeout to avoid hangs.
Example:
async with httpx.AsyncClient(timeout=self.timeout or 10.0) as client: ...
328-334: Set explicit HTTP timeouts for model listing callsDefault httpx timeouts can be high; wire self.timeout (or a sane default) into AsyncClient or per‑request to avoid UI stalls.
Example:
async with httpx.AsyncClient(timeout=self.timeout or 15.0) as client: tags_response = await client.get(url=tags_url, headers=self.headers) ...Also applies to: 347-351
371-376: Harden headers() for non‑string valuesSecretStrInput can technically hold non‑str; guard the type and strip safely.
- def headers(self) -> dict[str, str] | None: - """Get the headers for the Ollama API.""" - if self.api_key and self.api_key.strip(): - return {"Authorization": f"Bearer {self.api_key}"} - return None + def headers(self) -> dict[str, str] | None: + """Get the headers for the Ollama API.""" + key = self.api_key if isinstance(self.api_key, str) else None + if key and key.strip(): + return {"Authorization": f"Bearer {key.strip()}"} + return None
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/backend/base/langflow/initial_setup/starter_projects/News Aggregator.json(1 hunks)src/backend/tests/unit/components/languagemodels/test_chatollama_component.py(4 hunks)src/lfx/src/lfx/base/models/ollama_constants.py(1 hunks)src/lfx/src/lfx/components/ollama/ollama.py(9 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
src/backend/tests/unit/components/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
src/backend/tests/unit/components/**/*.py: Mirror the component directory structure for unit tests in src/backend/tests/unit/components/
Use ComponentTestBaseWithClient or ComponentTestBaseWithoutClient as base classes for component unit tests
Provide file_names_mapping for backward compatibility in component tests
Create comprehensive unit tests for all new components
Files:
src/backend/tests/unit/components/languagemodels/test_chatollama_component.py
{src/backend/**/*.py,tests/**/*.py,Makefile}
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code
Files:
src/backend/tests/unit/components/languagemodels/test_chatollama_component.py
src/backend/tests/unit/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
Test component integration within flows using create_flow, build_flow, and get_build_events utilities
Files:
src/backend/tests/unit/components/languagemodels/test_chatollama_component.py
src/backend/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
src/backend/tests/**/*.py: Unit tests for backend code must be located in the 'src/backend/tests/' directory, with component tests organized by component subdirectory under 'src/backend/tests/unit/components/'.
Test files should use the same filename as the component under test, with an appropriate test prefix or suffix (e.g., 'my_component.py' → 'test_my_component.py').
Use the 'client' fixture (an async httpx.AsyncClient) for API tests in backend Python tests, as defined in 'src/backend/tests/conftest.py'.
When writing component tests, inherit from the appropriate base class in 'src/backend/tests/base.py' (ComponentTestBase, ComponentTestBaseWithClient, or ComponentTestBaseWithoutClient) and provide the required fixtures: 'component_class', 'default_kwargs', and 'file_names_mapping'.
Each test in backend Python test files should have a clear docstring explaining its purpose, and complex setups or mocks should be well-commented.
Test both sync and async code paths in backend Python tests, using '@pytest.mark.asyncio' for async tests.
Mock external dependencies appropriately in backend Python tests to isolate unit tests from external services.
Test error handling and edge cases in backend Python tests, including using 'pytest.raises' and asserting error messages.
Validate input/output behavior and test component initialization and configuration in backend Python tests.
Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
Be aware of ContextVar propagation in async tests; test both direct event loop execution and 'asyncio.to_thread' scenarios to ensure proper context isolation.
Test error handling by mocking internal functions using monkeypatch in backend Python tests.
Test resource cleanup in backend Python tests by using fixtures that ensure proper initialization and cleanup of resources.
Test timeout and performance constraints in backend Python tests using 'asyncio.wait_for' and timing assertions.
Test Langflow's Messag...
Files:
src/backend/tests/unit/components/languagemodels/test_chatollama_component.py
src/backend/**/*component*.py
📄 CodeRabbit inference engine (.cursor/rules/icons.mdc)
In your Python component class, set the
iconattribute to a string matching the frontend icon mapping exactly (case-sensitive).
Files:
src/backend/tests/unit/components/languagemodels/test_chatollama_component.py
src/backend/**/components/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/icons.mdc)
In your Python component class, set the
iconattribute to a string matching the frontend icon mapping exactly (case-sensitive).
Files:
src/backend/tests/unit/components/languagemodels/test_chatollama_component.py
**/{test_*.py,*.test.ts,*.test.tsx}
📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)
**/{test_*.py,*.test.ts,*.test.tsx}: Check if tests have too many mock objects that obscure what’s actually being tested
Warn when mocks are used instead of testing real behavior and interactions
Suggest using real objects or simpler test doubles when mocks become excessive
Ensure mocks are used only for external dependencies, not core business logic
Recommend integration tests when unit tests become overly mocked
Check that test files follow the project’s naming conventions (backend: test_*.py; frontend: *.test.ts/tsx)
Verify that tests actually exercise the new or changed functionality, not placeholder assertions
Test files should have descriptive test function names explaining what is being tested
Organize tests logically with proper setup and teardown
Include edge cases and error conditions for comprehensive coverage
Verify tests cover both positive (success) and negative (failure) scenarios
Ensure tests are not mere smoke tests; they should validate behavior thoroughly
Ensure tests follow the project’s testing frameworks (pytest for backend, Playwright for frontend)
Files:
src/backend/tests/unit/components/languagemodels/test_chatollama_component.py
**/test_*.py
📄 CodeRabbit inference engine (coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt)
**/test_*.py: Backend tests must be named test_*.py and use proper pytest structure (fixtures, assertions)
For async backend code, use proper pytest async patterns (e.g., pytest-asyncio)
For API endpoints, include tests for both success and error responses
Files:
src/backend/tests/unit/components/languagemodels/test_chatollama_component.py
🧬 Code graph analysis (2)
src/backend/tests/unit/components/languagemodels/test_chatollama_component.py (2)
src/lfx/src/lfx/components/ollama/ollama.py (6)
ChatOllamaComponent(28-376)headers(372-376)build_model(163-230)is_valid_ollama_url(232-249)get_models(299-369)update_build_config(251-297)src/lfx/src/lfx/utils/util.py (1)
transform_localhost_url(119-161)
src/lfx/src/lfx/components/ollama/ollama.py (1)
src/lfx/src/lfx/inputs/inputs.py (1)
SecretStrInput(286-341)
🔇 Additional comments (15)
src/lfx/src/lfx/components/ollama/ollama.py (2)
41-66: API key input addition looks goodSecretStrInput is appropriate; naming and real_time_refresh align with the UI patterns. Icon string "Ollama" matches mapping.
214-216: Correctly plumbs headers into client via client_kwargsThis ensures ChatOllama receives Authorization when present without affecting local setups.
src/backend/tests/unit/components/languagemodels/test_chatollama_component.py (13)
406-408: Good: assert request URL via named kwargSwitching to call_args[1]["url"] matches how httpx is called.
Also applies to: 435-437
462-495: Headers property tests — clear and correctValidates None vs Authorization across cloud/local and api_key presence.
496-520: Cloud build_model passes headers via client_kwargs — OK
521-541: Cloud without API key — no client_kwargs — OK
542-560: Local build_model — no headers — OK
561-583: is_valid_ollama_url (cloud) includes headers — OK
584-603: is_valid_ollama_url (local) omits headers — OK
604-647: get_models (cloud) passes headers to GET and POST — OK
648-685: get_models (local) omits headers — OK
713-739: Local base_url clears/hides api_key — OK
741-766: Changing api_key triggers refresh — OK
767-774: Cloud URL isn’t transformed — OK
776-796: /v1 suffix stripped for cloud base_url — OK
adfc9e0 to
1ba9299
Compare
cc253cd to
8e47376
Compare
f71f1bc to
216b7ce
Compare
fix chat ollama tests for kwargs passed to http client chore: update component index and remove debugging prints chore: clean up comments rename base url field to be more consistent with other llm-provider components remove is_cloud_ollama check and allow headers for any host chore: remove commented code block remove _get_base_url property since transform_localhost_url returns original url if not local chore: update component index revert new aggregator google dep version to match upstream implement some coderabbit suggestions chore: update component index re-use headers in loop move ollama url import to top of test file and make ollama api key optional keep api_key field hidden and leave update_build_config logic unchanged and remove corresponding unit tests
b8a5efc to
eee38b8
Compare
| name="base_url", | ||
| display_name="Base URL", | ||
| info="Endpoint of the Ollama API. Defaults to http://localhost:11434 .", | ||
| display_name="Ollama API URL", |
There was a problem hiding this comment.
Is this the right terminology for a local connection though?
There was a problem hiding this comment.
good question, I'm not sure. Just wanted to stay consistent with the other llm model provider components, e.g., anthropic.py has the following
...
display_name="Anthropic API URL",
...But like you said Ollama is mainly used in a local environment at the moment. On the other hand this terminology helps to hint to the user that cloud Ollama is supported
| "template": self.template, | ||
| } | ||
| headers = self.headers | ||
| if headers is not None: |
There was a problem hiding this comment.
Is it the case that headers will/ should never be provided with a local instance?
There was a problem hiding this comment.
sadly (or maybe not since local can be configured to require api keys?) the headers can be passed to a local instance if the api key is a non-empty string, but I have confirmed that it works just fine regardless.
erichare
left a comment
There was a problem hiding this comment.
This is also a fantastic PR. LGTM @HzaRashid . I'm approving, but i did make one NIT comment + one question
Thank you so much for reviewing it! |
* feat: add cloud api support for chat ollama component fix chat ollama tests for kwargs passed to http client chore: update component index and remove debugging prints chore: clean up comments rename base url field to be more consistent with other llm-provider components remove is_cloud_ollama check and allow headers for any host chore: remove commented code block remove _get_base_url property since transform_localhost_url returns original url if not local chore: update component index revert new aggregator google dep version to match upstream implement some coderabbit suggestions chore: update component index re-use headers in loop move ollama url import to top of test file and make ollama api key optional keep api_key field hidden and leave update_build_config logic unchanged and remove corresponding unit tests * chore: update component index * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) * [autofix.ci] apply automated fixes (attempt 3/3) * Update component_index.json * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Hare <ericrhare@gmail.com>
Adds support for Ollama's cloud API with API key authentication. Unit tests added to verify header generation and authentication behavior.
Summary by CodeRabbit
New Features
Tests