feat: Allow openai-key to be retrieved via a function #10
Conversation
Enhance the handling of the openai-key variable. If it is a string it will be interpreted as the raw key. If it is a function then that function will be called to retrieve the key. This allows storing the key in some secure store (e.g. auth-source)
openai-chat.el
Outdated
| @@ -60,7 +60,7 @@ STREAM, STOP, MAX-TOKENS, PRESENCE-PENALTY, FREQUENCY-PENALTY, and LOGIT-BIAS." | |||
| (openai-request "https://api.openai.com/v1/chat/completions" | |||
| :type "POST" | |||
| :headers `(("Content-Type" . "application/json") | |||
| ("Authorization" . ,(concat "Bearer " key))) | |||
| ("Authorization" . ,(concat "Bearer " (openai--resolve-key key)))) | |||
There was a problem hiding this comment.
Ok, I also note that this would be required in the other repos that depend on openai, are you amenable to similar PRs in those repos
There was a problem hiding this comment.
I think perhaps moving the creation of the default headers into openai-request might be bettter, so the :headers could be removed from all those callsites ? I guess any headers that were specified could be merged on top.
There was a problem hiding this comment.
Ok, I also note that this would be required in the other repos that depend on openai, are you amenable to similar PRs in those repos
Other packages fully rely on this package, I don't think we will need to change anything from those packages.
I think perhaps moving the creation of the default headers into
openai-requestmight be bettter, so the:headerscould be removed from all those callsites ? I guess any headers that were specified could be merged on top.
This sounds reasonable. Some endpoints (mostly GET) don't require the key; but now, every endpoint seems to require it. 😕 As of right now, I might prefer to leave it as it is since I do not know when they are going to change it (require key or don't require key). WDYT?
There was a problem hiding this comment.
Ok, I also note that this would be required in the other repos that depend on openai, are you amenable to similar PRs in those repos
The call to resolve key should move to inside openai-request - then no call sites need to change, but this is awkward since the key has already been put in a Authorization header by this point. By moving the creation of the authorization header into openai-request we can tidy that up.
sw1nn
force-pushed
the
openai-key-from-auth-source
branch
from
3622942 to
d835351
Compare
|
baba8fc moves the headers construction around, but the change is getting bigger now than I envisioned at first. It somewhat cleans up all the calls to |
sw1nn
force-pushed
the
openai-key-from-auth-source
branch
2 times, most recently
from
fb4785c to
7b4474e
Compare
All the callers of openai-request pass the Content-Type, Authorization
headers and specify the parser as json-read.
This change makes the defaults be
:headers `(("Content-Type" . "application/json")
- ("Authorization" . ,(concat "Bearer " (openai--resolve-key openai-key)))
:parser 'json-read
This is allow the key resolution call to be common (and private).
sw1nn
force-pushed
the
openai-key-from-auth-source
branch
from
7b4474e to
5c26a3c
Compare
Need to rework the macro a little to ensure that the openai-key is bound at runtime not compile time. TODO - does this even need to be a macro?
|
Change has become bigger than I thought - I'll keep chipping away in my local repo, but probably not worth a PR until the major restructuring discussed in #5 is fleshed out. Closing. |
|
Sorry for not being able to merge this feature request! I will make sure to add this feature once we've done restructuring the code base! 😓 |
|
I've used some of your code, and the feature is integrated in #13. Hope you like it! :D |
Enhance the handling of the openai-key variable. If it is a string it will be interpreted as the raw key. If it is a function then that function will be called to retrieve the key. This allows storing the key in some secure store (e.g. auth-source)