-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Ok, well, that got out of hand #3718
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
Changes from all commits
d409f69
846982f
ba816bf
2ea4a1b
e4ee3d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -255,7 +255,7 @@ async fn update_agent_provider( | |
| .get_param("GOOSE_MODEL") | ||
| .expect("Did not find a model on payload or in env to update provider with") | ||
| }); | ||
| let model_config = ModelConfig::new(model); | ||
| let model_config = ModelConfig::new(&model).map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the line above where we have .expect() will kill goosed and it is all over. not sure how we would have gotten here, but still, we should probably do another 500; I also think we should change the description of the 500 from internal server error here to something descriptive; could not update provider, make sure that you have the right config or something |
||
| let new_provider = create(&payload.provider, model_config).unwrap(); | ||
| agent | ||
| .update_provider(new_provider) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,3 +22,5 @@ pub mod utils; | |
|
|
||
| #[cfg(test)] | ||
| mod cron_test; | ||
| #[macro_use] | ||
| mod macros; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| #[macro_export] | ||
| macro_rules! impl_provider_default { | ||
| ($provider:ty) => { | ||
| impl Default for $provider { | ||
| fn default() -> Self { | ||
| let model = $crate::model::ModelConfig::new( | ||
| &<$provider as $crate::providers::base::Provider>::metadata().default_model, | ||
| ) | ||
| .expect(concat!( | ||
| "Failed to create model config for ", | ||
| stringify!($provider) | ||
| )); | ||
|
|
||
| <$provider>::from_env(model) | ||
| .expect(concat!("Failed to initialize ", stringify!($provider))) | ||
| } | ||
| } | ||
| }; | ||
| } |
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.
we probably just want to skip this model if we can't load it