refactor(2671): move retry logic and implement tokio_retry #2674
Conversation
github-actions
bot
added
the
type: chore
|
duplicate of #2673 |
onyedikachi-david
changed the title
Not at all, if not for slow compilation on my laptop i would have submitted before now. @ssddOnTop |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2674 +/- ##
==========================================
- Coverage 86.31% 86.25% -0.07%
==========================================
Files 255 255
Lines 24673 24691 +18
==========================================
Hits 21297 21297
- Misses 3376 3394 +18 ☔ View full report in Codecov by Sentry. |
src/cli/llm/wizard.rs
Outdated
| { | ||
| Ok(response) => Ok(A::try_from(response)?), | ||
| Err(genai::Error::WebModelCall { webc_error, .. }) => { | ||
| if webc_error.to_string().contains("429") { |
There was a problem hiding this comment.
Pattern match instead of performing a string comparison — webc::Error::ResponseFailedStatus { status: 429, body: _ }
There was a problem hiding this comment.
The webc module is not publicly exposed, so i cant match with it.
There was a problem hiding this comment.
I've gotten a workaround. I'll make a push soon.
src/cli/llm/wizard.rs
Outdated
| }))?) | ||
| } | ||
| } | ||
| Err(e) => Ok(Err(Error::GenAI(e))?), |
There was a problem hiding this comment.
We should just check for the condition here that the status code is 429
|
Moving to draft to reduce noise and improve CI efficiency. Once you are ready just mark it as "ready to review". Feel free to give a shoutout on the #contributors channel on Discord if you want immediate attention. |
|
Action required: PR inactive for 5 days. |
github-actions
bot
added
state: inactive
|
Action required: PR inactive for 5 days. |
|
Moving to draft to reduce noise and improve CI efficiency. Once you are ready just mark it as "ready to review". Feel free to give a shoutout on the #contributors channel on Discord if you want immediate attention. |
Co-authored-by: Tushar Mathur <[email protected]>
Signed-off-by: David Anyatonwu <[email protected]>
| impl From<genai::Error> for Error { | ||
| fn from(err: genai::Error) -> Self { |
There was a problem hiding this comment.
You might not need so much code since this is merged — jeremychone/rust-genai@736bbec
There was a problem hiding this comment.
@tusharmath Pls, why is this repo: https://github.dev/laststylebender14/rust-genai being used instead of the main genai repo: https://github.com/jeremychone/rust-genai
jeremychone's latest lib.rs:
// region: --- Modules
mod support;
mod client;
mod common;
mod error;
// -- Flatten
pub use client::*;
pub use common::*;
pub use error::{Error, Result};
// -- Public Modules
pub mod adapter;
pub mod chat;
pub mod resolver;
pub mod webc;
// endregion: --- Moduleslaststylebender14's latest commit hash lib.rs:
// region: --- Modules
mod support;
mod webc;
mod client;
mod common;
mod error;
// -- Flatten
pub use client::*;
pub use common::*;
pub use error::{Error, Result};
// -- Public Modules
pub mod adapter;
pub mod chat;
pub mod resolver;
// endregion: --- ModulesSo the webc is still private.
There was a problem hiding this comment.
@laststylebender14 can you update your Repo with the latest changes? Also can you transfer ownership to tailcallhq
There was a problem hiding this comment.
@onyedikachi-david I have taken the latest changes here — https://github.com/tailcallhq/rust-genai
There was a problem hiding this comment.
please apply discussed changes:
- use https://github.com/tailcallhq/rust-genai as crate
- use automatic conversion from webc::Error
|
Action required: PR inactive for 5 days. |
github-actions
bot
added
the
state: inactive
|
PR closed after 10 days of inactivity. |
github-actions
bot
removed
the
state: inactive
|
Action required: PR inactive for 5 days. |
github-actions
bot
added
the
state: inactive
| .exec_chat(self.model.as_str(), q.try_into()?, None) | ||
| .await?; | ||
| A::try_from(response) | ||
| let retry_strategy = ExponentialBackoff::from_millis(1000).map(jitter).take(5); |
There was a problem hiding this comment.
the base for the strategy is too high, please, check the doc, it'll be 1 sec -> 16 minutes -> 277 hours -> ... in that case.
Consider:
- change the base the way
base ^ attemptis reasonable - you may use the
factoroption to simplify the math - specify
max_delayto limit maximum delay - don't use jitter, it adds randomness making it harder to mitigate 429 status
| let mut delay = 3; | ||
| loop { |
| impl From<genai::Error> for Error { | ||
| fn from(err: genai::Error) -> Self { |
There was a problem hiding this comment.
please apply discussed changes:
- use https://github.com/tailcallhq/rust-genai as crate
- use automatic conversion from webc::Error
|
Hey @onyedikachi-david thanks for the pr. Would you able to finish this based on the discussions? |
github-actions
bot
removed
the
state: inactive
Okay. I'll push the needed changes in few hours time. |
|
webc isn't public in this repo: https://github.com/tailcallhq/rust-genai; hence I can't implement some of the requested changes. |
|
Action required: PR inactive for 5 days. |
github-actions
bot
added
the
state: inactive
|
PR closed after 10 days of inactivity. |
Summary:
This refactor enhances the of API calls by leveraging tokio_retry
for automatic retries with an exponential backoff strategy.
Issue Reference(s):
Fixes #2671
/claim #2671
Build & Testing:
cargo testsuccessfully../lint.sh --mode=fixto fix all linting issues raised by./lint.sh --mode=check.Checklist:
<type>(<optional scope>): <title>