-
Notifications
You must be signed in to change notification settings - Fork 32
Rename chat_completions
config to openai
#438
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Mateus Devino <[email protected]>
pub chat_completions: Option<OpenAiConfig>, | ||
/// Completions service and associated configuration, can be omitted if configuring for chat generation is not wanted | ||
pub completions: Option<OpenAiConfig>, | ||
pub openai: Option<OpenAiConfig>, |
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.
nice, even this one is lining up well with this name
@@ -229,7 +227,7 @@ impl OrchestratorConfig { | |||
})?; | |||
if config_yaml.contains("chat_generation") { | |||
warn!( | |||
"`chat_generation` is deprecated and will be removed in 1.0. Rename it to `chat_completions`." | |||
"`chat_generation` is deprecated and will be removed in 1.0. Rename it to `openai`." |
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.
for precaution's sake. Lets also mention chat_completion
being deprecated as well.
@@ -124,12 +124,11 @@ async fn no_detectors() -> Result<(), anyhow::Error> { | |||
}); | |||
|
|||
// Start orchestrator server and its dependencies | |||
let mut mock_chat_completions_server = | |||
MockServer::new("chat_completions").with_mocks(chat_mocks); | |||
let mut mock_openai_server = MockServer::new("chat_completions").with_mocks(chat_mocks); |
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.
what to call the name of the MockServer
as chat_completions
still instead of openai
?
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.
Because these are tests for chat completions. This name is really used for anything, but I think leaving it as is makes things* clearer.
With the upcoming work to support Open AI's completions endpoint, we should rename the orchestrator config file entry to
openai
, as it will be used for other endpoints rather than just chat completions.