-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Generic retry and error parsing #3558
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
0a7fbde
c6fd1bf
f08ec24
b60d032
c215f99
b092746
dbbaad0
a4d3f58
1f31c76
c9145e4
d2ab2fc
b542699
1d84913
fe0365e
6582658
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 |
|---|---|---|
|
|
@@ -226,6 +226,8 @@ async fn get_tools( | |
| path = "/agent/update_provider", | ||
| responses( | ||
| (status = 200, description = "Update provider completed", body = String), | ||
| (status = 400, description = "Bad request - missing or invalid parameters"), | ||
| (status = 401, description = "Unauthorized - invalid secret key"), | ||
| (status = 500, description = "Internal server error") | ||
| ) | ||
| )] | ||
|
|
@@ -234,29 +236,26 @@ async fn update_agent_provider( | |
| headers: HeaderMap, | ||
| Json(payload): Json<UpdateProviderRequest>, | ||
| ) -> Result<StatusCode, StatusCode> { | ||
| // Verify secret key | ||
| let secret_key = headers | ||
| .get("X-Secret-Key") | ||
| .and_then(|value| value.to_str().ok()) | ||
| .ok_or(StatusCode::UNAUTHORIZED)?; | ||
|
|
||
| if secret_key != state.secret_key { | ||
| return Err(StatusCode::UNAUTHORIZED); | ||
| } | ||
| verify_secret_key(&headers, &state)?; | ||
|
|
||
| let agent = state | ||
| .get_agent() | ||
| .await | ||
| .map_err(|_| StatusCode::PRECONDITION_FAILED)?; | ||
|
|
||
| let config = Config::global(); | ||
| let model = payload.model.unwrap_or_else(|| { | ||
| config | ||
| .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).map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; | ||
| let new_provider = create(&payload.provider, model_config).unwrap(); | ||
| let model = match payload | ||
| .model | ||
| .or_else(|| config.get_param("GOOSE_MODEL").ok()) | ||
| { | ||
| Some(m) => m, | ||
| None => return Err(StatusCode::BAD_REQUEST), | ||
| }; | ||
|
|
||
| let model_config = ModelConfig::new(&model).map_err(|_| StatusCode::BAD_REQUEST)?; | ||
|
Collaborator
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. would be nice if we could include text with these, but maybe leave that for another change
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. I do agree - I didn't quickly see how that works though. there's a whole bunch of other handlers that would be helped |
||
|
|
||
| let new_provider = | ||
| create(&payload.provider, model_config).map_err(|_| StatusCode::BAD_REQUEST)?; | ||
| agent | ||
| .update_provider(new_provider) | ||
| .await | ||
|
|
||
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.
this shows up as:
is that intended when not debug?
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 don't know - I didn't really touch this code, the new linter rules force me to break up this function
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.
ah - fair enough, yes refactoring makes reviewing diffs very difficult when it breaks up things (wish the diff was a bit smarter, and could point out the code previously existed - if you search for it you can see but easy to miss). Yeah makes sense, just odd I never saw this before.