feat: add extra_headers to GenerateInputs #85
Conversation
|
|
||
| extra_headers = {"RITS_API_KEY": os.environ.get("OPENAI_API_KEY")} | ||
| ret = await be( | ||
| GenerateInputs(prompt="what is up?", n=3, extra_headers=extra_headers) |
There was a problem hiding this comment.
This pattern does not make sense to me. How would this extra_headers value get into the GenerateInputs object? Are we expecting user code to add these headers to the generate_inputs field of every ChatCompletionInputs that this code creates? Why not pass the extra headers to the constructor for the backend object?
There was a problem hiding this comment.
extra_headers is a supported litellm and openai parameter just like the rest of our GenerateInputs:
https://github.com/BerriAI/litellm/blob/main/litellm/llms/openai/completion/transformation.py#L143
https://github.com/openai/openai-python/blob/main/src/openai/resources/completions.py#L75
So based on the design of GenerationInputs starting with LiteLLM params, I think this fits. I added it here:
When **inputs is applied to the completion calls, it just works for both openai and litellm completions.
I don't have a use case that requires yet another param for backend construction (although I can see why this could have been). The extra_headers support in the litellm and openai adds another useful param for granite-io users and also cleans up this issue where I had some special-case code for RITS. Now that can be done as part of extra_headers.
There was a problem hiding this comment.
@frreiss Oh I see. I'm doing this all from the backend perspective and not from an I/O processor. I'll take another look at that.
There was a problem hiding this comment.
@frreiss I made the openai backend forward a default_headers client contructor param. This is only applicable for openai but fits the pattern you mentioned.
Litellm doesn't follow that pattern. No construct-time params are used -- just generate time. This still works as the original approach I followed using generate params that are openai+litellm (defacto standard).
Not planning to make either of these work where they are not wanted (e.g. transformers or stuff default_headers into liteLLM) but we could backlog that if you think we should be doing that.
There was a problem hiding this comment.
Not a big deal, I just won't use this parameter if it isn't an argument of the constructor for the various subclasses of Backend. If I need such functionality, I'll add it as an argument of the constructor for the appropriate subclass.
* extra_headers works for openai and litellm and can be used to get rid of some code specific to an internal provider header. * Test is skipped, but can be used for manual testing against altenate server to prove effectiveness * Env vars that are used in the test are the standard ones already recognized by the client. Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
* openai users can use aConfig to pass default_headers to the openai client as would be expected * default_headers does not apply to other backend * extra_headers as a generate param works for both openai and litellm These are normal params for these clients. No renaming or swizzling. Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Closes: #39