Custom headers for OpenAPI compatible providers - #45673
Conversation
Some enterprise instances dont use the standard "Authorization" header. Custom headers are required in those cases to enable authentication. So add them as an option for OpenAI compatible providers. Signed-off-by: Aurabindo Pillai <mail@aurabindo.in>
ApiEndpointNotFound error does not print the provider name. Print it so that the error message is more helpful. Signed-off-by: Aurabindo Pillai <mail@aurabindo.in>
Enable modifying custom headers in AI settings from the UI Signed-off-by: Aurabindo Pillai <mail@aurabindo.in>
|
Hi, any chance you folks could look at this again and make the final changes? Supporting custom API header indeed would be awesome for enterprise settings. Much appreciated! |
danilo-leal
left a comment
There was a problem hiding this comment.
Hey @aurabindo, thanks for the PR and sorry for the late response here! In testing this out, I found some fixes we need to make before we're ready to merge (aside from the conflict this branch now has):
- The input tab order in the "Add LLM Provider" modal is wrong: tabbing from the "Header Name" input gets me to the model name input, as opposed to the "Header Value" input.
- We should also add the "Remove Header" button as part of the tab index
- We should make "Header Name" and "Header Value" the label for the corresponding inputs and add in the placeholder slot an example of content that can be inserted in those inputs
- After I hit "Save Provider" in the modal, I still see the input to add the API key, as well as the custom HTTP headers in the settings view of the agent panel. This feels wrong to me; I'd expect everything to be filled already (image below).
After we push these changes, we can fine-tune the UI more. Let me know if you're still interested in working on this, otherwise, I might either close it for now or work on it later time-permitting. Thank you!
|
Hello! Would it be better to open a new PR with this, or should we wait to see if this one gets updated? |
|
Given the merge conflicts I think it is best to have a new PR |
Related discussion: #42663
Release Notes: