feat(ollama): add bearer token as optional setting#165325
feat(ollama): add bearer token as optional setting#165325ziouf wants to merge 6 commits intohome-assistant:devfrom
Conversation
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Hey there @synesthesiam, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
Adds support for authenticating to an Ollama endpoint using an optional Bearer token, intended for setups (e.g., proxies) that require an Authorization header.
Changes:
- Add
api_keyfield to the Ollama config flow UI strings. - Introduce
CONF_API_KEYand attempt to use it to pass anAuthorization: Bearer ...header when creating the Ollama client. - Update integration setup to also pass the optional Authorization header at runtime.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
homeassistant/components/ollama/strings.json |
Adds UI label for an API key field in the config flow. |
homeassistant/components/ollama/const.py |
Introduces CONF_API_KEY constant. |
homeassistant/components/ollama/config_flow.py |
Attempts to read api_key from user input and pass Authorization headers during connection validation. |
homeassistant/components/ollama/__init__.py |
Passes optional Authorization headers when creating the runtime ollama.AsyncClient. |
38ca91d to
c07595a
Compare
joostlek
left a comment
There was a problem hiding this comment.
Okay so the Ollama integration integrates with Ollama. In the issue linked, people talk about running a proxy in front of Ollama which requires the auth. So I don't think this is something the integration should be concerned about. Now bearer tokens are used, tomorrow basic auth, and another day later someone comes with OAuth, which would grow the scope of the integration, while from what I understand, Ollama doesn't support it.
So I personally think we should close this PR as from what I understand, this isn't a feature offered by Ollama
It's very strange reasoning to close a PR that implements a feature missing from the integration on the pretext that one day someone might ask to add another feature that is unrelated to this PR... I personally need to be able to connect with Ollama's bearer token. The issue linked in the PR mentioned the same need, in my opinion. So I linked it as a courtesy. If, one day, someone ever asks to integrate an authentication feature that is not natively supported by ollama, you will be perfectly justified in refusing. |
This is my first contribution to this wonderful project. Please do not hesitate to point out any mistakes I may have made.
Proposed change
Implements ollama bearer token as an optional ollama integration setting
As described here : https://docs.ollama.com/api/authentication#api-keys
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: