-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Anthropic (claude) #2403
base: main
Are you sure you want to change the base?
Use Anthropic (claude) #2403
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2403 +/- ##
=======================================
Coverage 89.79% 89.79%
=======================================
Files 314 314
Lines 10839 10839
=======================================
Hits 9733 9733
Misses 1106 1106 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey i'd love resolution on the key name on line 102 of bootstrap. once that's clear happy to merge
lib/lightning/config/bootstrap.ex
Outdated
@@ -99,7 +99,7 @@ defmodule Lightning.Config.Bootstrap do | |||
:string, | |||
Utils.get_env([:lightning, :apollo, :endpoint]) | |||
), | |||
openai_api_key: env!("OPENAI_API_KEY", :string, nil) | |||
openai_api_key: env!("ANTHROPIC_API_KEY", :string, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should the key name here be more generic? maybe change openai_api_key
to apollo_3rd_party_key
? or am i misunderstanding this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question.
It certainly isn't a generic apollo API key. Different apollo services require different API keys.
It might make sense to rename to AI_ASSISTANT_API_KEY
though. That way we're binding it to the service, rather than the implementation. Of course it's also less declarative from a devops point of view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to change either way if you have a strong view. I don't. We'll know more when we add more services I think - although of course that might take a while.
Hi @taylordowns2000 and @stuartc where are we sitting on this? What do we want to do with the env var name? |
0883dd7
to
99b2a57
Compare
@stuartc friendly reminder we need to deal with this! |
This PR renames
OPENAI_API_KEY
toAI_ASSISTANT_API_KEY
, to reflect an upcoming change on the apollo backend.Note that this isn't technically needed to start using the new model. Setting the anthropic key values into the open AI key will work just fine.