xhcoding / emacs-aichat

AI Chat in Emacs, including OpenAI and Bing Chat
61 stars 9 forks source link

Defer auth-source-pick-first-password call #11

Closed MunsterPlop closed 1 year ago

MunsterPlop commented 1 year ago
xhcoding commented 1 year ago

This code patch will cause errors where it is called

emacs calls this function at startup because the following variable definition calls it

https://github.com/xhcoding/emacs-aichat/blob/153f92d87f53908fe96e5ef99945bdc92ece87b8/aichat-openai.el#L112-L114

Maybe we should change defvar to defun, and get the key at runtime instead of at require time

MunsterPlop commented 1 year ago

Indeed, I overlooked this. That's what happens when you stay up too long. I'm gonna sleep for now and will look into that tomorrow.

Another extension I contribute to stores the api key in a variable openai-key which is used as is if it's a string or is called using funcall if it's a function. You could then pass (lambda () (auth-source-pick-first-password :host "platform.openai.com" :user "aichat-openai")).

MunsterPlop commented 1 year ago

Alright, I couldn't help but put something together before sleeping so here's my suggested change:

(defcustom aichat-openai-api-key nil
  "OpenAI key as a string or a function that loads and returns it."
  :type '(choice (function :tag "Function")
                 (string :tag "String"))
  :group 'aichat-openai)

Turn aichat-openai--http-headers into:

(defun aichat-openai--make-http-headers ()
  "Create http headers to send in requests to the API."
  `(("Content-Type" . "application/json")
    ("Authorization" . ,(format "Bearer %s" (if (functionp aichat-openai-api-key)
                                                (funcall aichat-openai-api-key)
                                              aichat-openai-api-key)))))

Then replace

          :headers aichat-openai--http-headers

with

          :headers (aichat-openai--make-http-headers)

It would also require updating the documentation to mention you have to explicitely set aichat-openai-api-key.

There's obviously some error handling to do when the api-key hasn't been set. I will take care of that tomorrow if you're okay with the whole idea.

xhcoding commented 1 year ago

Thanks, I appreciate that and think it's ok for me.

MunsterPlop commented 1 year ago

Hi again,

In the end, I moved the lambda in its own function:

(defun aichat-openai--default-api-key-function ()
  "Fetch the API key with auth-source."
  (auth-source-pick-first-password :host "platform.openai.com"))

I also thought it would be nice not to break the current users' config so I also kept the default :host mentioned in the README.

If there's anything you'd like to add, I'll be around.

xhcoding commented 1 year ago

Thanks for your patch.

I don't usually use OpenAI, so there are only two demo functions showing API calls in emacs-openai.el. If you are an OpenAI user, please feel free to submit a PR to implement useful or interesting features using OpenAI.