Skip to content

Commit 21368c5

Browse files
committed
chore(hf): remove redundant retry logic
1 parent 839455a commit 21368c5

File tree

5 files changed

+18
-86
lines changed

5 files changed

+18
-86
lines changed

core/Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/services/hf/Cargo.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,16 @@ all-features = true
3232

3333
[dependencies]
3434
async-trait = "0.1"
35-
backon = "1.6"
3635
base64 = { workspace = true }
3736
bytes = { workspace = true }
3837
futures = { workspace = true }
3938
http = { workspace = true }
4039
log = { workspace = true }
4140
opendal-core = { path = "../../core", version = "0.55.0", default-features = false }
4241
percent-encoding = "2"
43-
reqwest = { version = "0.12", default-features = false, features = ["rustls-tls"] }
42+
reqwest = { version = "0.12", default-features = false, features = [
43+
"rustls-tls",
44+
] }
4445
serde = { workspace = true, features = ["derive"] }
4546
serde_json = { workspace = true }
4647
subxet = { git = "https://github.com/kszucs/subxet" }

core/services/hf/src/backend.rs

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -120,15 +120,6 @@ impl HfBuilder {
120120
}
121121
self
122122
}
123-
124-
/// Set the maximum number of retries for commit operations.
125-
///
126-
/// Retries on commit conflicts (HTTP 412) and transient server
127-
/// errors (HTTP 5xx). Default is 3.
128-
pub fn max_retries(mut self, max_retries: usize) -> Self {
129-
self.config.max_retries = Some(max_retries);
130-
self
131-
}
132123
}
133124

134125
impl Builder for HfBuilder {
@@ -192,18 +183,8 @@ impl Builder for HfBuilder {
192183
let repo = HfRepo::new(repo_type, repo_id, Some(revision.clone()));
193184
debug!("backend repo uri: {:?}", repo.uri(&root, ""));
194185

195-
let max_retries = self.config.max_retries.unwrap_or(3);
196-
debug!("backend max_retries: {}", max_retries);
197-
198186
Ok(HfBackend {
199-
core: Arc::new(HfCore::build(
200-
info,
201-
repo,
202-
root,
203-
token,
204-
endpoint,
205-
max_retries,
206-
)?),
187+
core: Arc::new(HfCore::build(info, repo, root, token, endpoint)?),
207188
})
208189
}
209190
}
@@ -287,15 +268,13 @@ pub(super) mod test_utils {
287268
}
288269

289270
/// Operator for a private dataset requiring HF_OPENDAL_DATASET and HF_OPENDAL_TOKEN.
290-
/// Uses higher max_retries to tolerate concurrent commit conflicts (412).
291271
pub fn testing_operator() -> Operator {
292272
let (repo_id, token) = testing_credentials();
293273
let op = Operator::new(
294274
HfBuilder::default()
295275
.repo_type("dataset")
296276
.repo_id(&repo_id)
297-
.token(&token)
298-
.max_retries(10),
277+
.token(&token),
299278
)
300279
.unwrap()
301280
.finish();
@@ -309,8 +288,7 @@ pub(super) mod test_utils {
309288
HfBuilder::default()
310289
.repo_type("bucket")
311290
.repo_id(&repo_id)
312-
.token(&token)
313-
.max_retries(10),
291+
.token(&token),
314292
)
315293
.unwrap()
316294
.finish();

core/services/hf/src/config.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,6 @@ pub struct HfConfig {
5252
///
5353
/// Default is "https://huggingface.co".
5454
pub endpoint: Option<String>,
55-
/// Maximum number of retries for commit operations.
56-
///
57-
/// Retries on commit conflicts (HTTP 412) and transient server
58-
/// errors (HTTP 5xx). Default is 3.
59-
pub max_retries: Option<usize>,
6055
}
6156

6257
impl Debug for HfConfig {

core/services/hf/src/core.rs

Lines changed: 12 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
use std::fmt::Debug;
1919
use std::sync::Arc;
2020

21-
use backon::ExponentialBuilder;
22-
use backon::Retryable;
2321
use bytes::Buf;
2422
use bytes::Bytes;
2523
use http::Request;
@@ -219,7 +217,6 @@ pub struct HfCore {
219217
pub root: String,
220218
pub token: Option<String>,
221219
pub endpoint: String,
222-
pub max_retries: usize,
223220

224221
/// HTTP client with redirects disabled, used by XET probes to
225222
/// inspect headers on 302 responses.
@@ -243,7 +240,6 @@ impl HfCore {
243240
root: String,
244241
token: Option<String>,
245242
endpoint: String,
246-
max_retries: usize,
247243
no_redirect_client: HttpClient,
248244
) -> Self {
249245
Self {
@@ -252,7 +248,6 @@ impl HfCore {
252248
root,
253249
token,
254250
endpoint,
255-
max_retries,
256251
no_redirect_client,
257252
}
258253
}
@@ -267,21 +262,12 @@ impl HfCore {
267262
root: String,
268263
token: Option<String>,
269264
endpoint: String,
270-
max_retries: usize,
271265
) -> Result<Self> {
272266
let standard = HttpClient::with(build_reqwest(reqwest::redirect::Policy::default())?);
273267
let no_redirect = HttpClient::with(build_reqwest(reqwest::redirect::Policy::none())?);
274268
info.update_http_client(|_| standard);
275269

276-
Ok(Self::new(
277-
info,
278-
repo,
279-
root,
280-
token,
281-
endpoint,
282-
max_retries,
283-
no_redirect,
284-
))
270+
Ok(Self::new(info, repo, root, token, endpoint, no_redirect))
285271
}
286272

287273
/// Build an authenticated HTTP request.
@@ -304,30 +290,14 @@ impl HfCore {
304290
self.repo.uri(&self.root, path)
305291
}
306292

307-
/// Send a request with retries, returning the successful response.
308-
///
309-
/// Retries on commit conflicts (HTTP 412) and transient server errors
310-
/// (HTTP 5xx) up to `self.max_retries` attempts with exponential backoff.
293+
/// Send a request and return the successful response or a parsed error.
311294
pub(super) async fn send(&self, req: Request<Buffer>) -> Result<Response<Buffer>> {
312-
let backoff = ExponentialBuilder::default()
313-
.with_min_delay(std::time::Duration::from_millis(200))
314-
.with_max_delay(std::time::Duration::from_millis(6400))
315-
.with_max_times(self.max_retries.saturating_sub(1));
316-
let client = self.info.http_client();
317-
318-
let send_once = || async {
319-
let resp = client.send(req.clone()).await?;
320-
if resp.status().is_success() {
321-
Ok(resp)
322-
} else {
323-
Err(parse_error(resp))
324-
}
325-
};
326-
327-
send_once
328-
.retry(backoff)
329-
.when(|e: &Error| e.kind() == ErrorKind::ConditionNotMatch || e.is_temporary())
330-
.await
295+
let resp = self.info.http_client().send(req).await?;
296+
if resp.status().is_success() {
297+
Ok(resp)
298+
} else {
299+
Err(parse_error(resp))
300+
}
331301
}
332302

333303
/// Send a request, check for success, and deserialize the JSON response.
@@ -402,16 +372,11 @@ impl HfCore {
402372
.body(Buffer::new())
403373
.map_err(new_request_build_error)?;
404374

405-
let mut attempt = 0;
406-
let resp = loop {
407-
let resp = self.no_redirect_client.send(req.clone()).await?;
375+
let resp = self.no_redirect_client.send(req).await?;
408376

409-
attempt += 1;
410-
let retryable = resp.status().is_server_error();
411-
if attempt >= self.max_retries || !retryable {
412-
break resp;
413-
}
414-
};
377+
if resp.status().is_client_error() || resp.status().is_server_error() {
378+
return Err(parse_error(resp));
379+
}
415380

416381
let hash = resp
417382
.headers()
@@ -434,11 +399,6 @@ impl HfCore {
434399
Ok(Some(XetFileInfo::new(hash.to_string(), size)))
435400
}
436401

437-
/// Commit file changes (uploads and/or deletions) to the repository.
438-
///
439-
/// Retries on commit conflicts (HTTP 412) and transient server errors
440-
/// (HTTP 5xx), matching the behavior of the official HuggingFace Hub
441-
/// client.
442402
/// Determine upload mode by calling the preupload API.
443403
///
444404
/// Returns the upload mode string from the API (e.g., "regular" or "lfs").
@@ -626,7 +586,6 @@ pub(crate) mod test_utils {
626586
"/".to_string(),
627587
None,
628588
endpoint.to_string(),
629-
3,
630589
HttpClient::with(mock_client.clone()),
631590
);
632591

0 commit comments

Comments
 (0)