This is an automated email from the ASF dual-hosted git repository.
chitralverma pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/opendal.git
The following commit(s) were added to refs/heads/main by this push:
new cbba56193 fix(bindings/python): validate RetryLayer numeric inputs
(#7879)
cbba56193 is described below
commit cbba561933b729050c51ae88aff5eb6b3febceac
Author: Chitral Verma <[email protected]>
AuthorDate: Tue Jul 7 20:31:44 2026 +0530
fix(bindings/python): validate RetryLayer numeric inputs (#7879)
RetryLayer accepted out-of-range values and forwarded them to the core
backoff builder, where they produce degenerate behavior: factor < 1.0
(or
NaN/inf) yields a nonsensical backoff, and negative/NaN max_delay or
min_delay silently clamp to zero via the f64->u64 cast.
Reject these at construction with ConfigInvalid: factor must be finite
and
>= 1.0; max_delay and min_delay must be finite and non-negative.
Document
the ranges and add tests covering the boundaries.
---
bindings/python/python/opendal/layers.pyi | 14 ++++--
bindings/python/src/layers/retry.rs | 29 +++++++++--
bindings/python/tests/layers/test_retry_layer.py | 64 ++++++++++++++++++++++++
3 files changed, 99 insertions(+), 8 deletions(-)
diff --git a/bindings/python/python/opendal/layers.pyi
b/bindings/python/python/opendal/layers.pyi
index 1cb4482a6..728b58e87 100644
--- a/bindings/python/python/opendal/layers.pyi
+++ b/bindings/python/python/opendal/layers.pyi
@@ -126,15 +126,23 @@ class RetryLayer(Layer):
max_times : Optional[int]
Maximum number of retry attempts. Defaults to ``3``.
factor : Optional[float]
- Backoff factor applied between retries. Defaults to ``2.0``.
+ Backoff factor applied between retries. Must be a finite value
+ ``>= 1.0``. Defaults to ``2.0``.
jitter : bool
Whether to apply jitter to the backoff. Defaults to ``False``.
max_delay : Optional[float]
- Maximum delay (in seconds) between retries. Defaults to ``60.0``.
+ Maximum delay (in seconds) between retries. Must be finite and
+ non-negative. Defaults to ``60.0``.
min_delay : Optional[float]
- Minimum delay (in seconds) between retries. Defaults to ``1.0``.
+ Minimum delay (in seconds) between retries. Must be finite and
+ non-negative. Defaults to ``1.0``.
Returns
-------
RetryLayer
+
+ Raises
+ ------
+ ConfigInvalid
+ If ``factor``, ``max_delay``, or ``min_delay`` is out of range.
"""
diff --git a/bindings/python/src/layers/retry.rs
b/bindings/python/src/layers/retry.rs
index 2376fd0c7..dc1f9806a 100644
--- a/bindings/python/src/layers/retry.rs
+++ b/bindings/python/src/layers/retry.rs
@@ -51,17 +51,25 @@ impl RetryLayer {
/// max_times : Optional[int]
/// Maximum number of retry attempts. Defaults to ``3``.
/// factor : Optional[float]
- /// Backoff factor applied between retries. Defaults to ``2.0``.
+ /// Backoff factor applied between retries. Must be a finite value
+ /// ``>= 1.0``. Defaults to ``2.0``.
/// jitter : bool
/// Whether to apply jitter to the backoff. Defaults to ``False``.
/// max_delay : Optional[float]
- /// Maximum delay (in seconds) between retries. Defaults to ``60.0``.
+ /// Maximum delay (in seconds) between retries. Must be finite and
+ /// non-negative. Defaults to ``60.0``.
/// min_delay : Optional[float]
- /// Minimum delay (in seconds) between retries. Defaults to ``1.0``.
+ /// Minimum delay (in seconds) between retries. Must be finite and
+ /// non-negative. Defaults to ``1.0``.
///
/// Returns
/// -------
/// RetryLayer
+ ///
+ /// Raises
+ /// ------
+ /// ConfigInvalid
+ /// If ``factor``, ``max_delay``, or ``min_delay`` is out of range.
#[new]
#[pyo3(signature = (
max_times = None,
@@ -82,16 +90,27 @@ impl RetryLayer {
retry = retry.with_max_times(max_times);
}
if let Some(factor) = factor {
+ if !factor.is_finite() || factor < 1.0 {
+ return Err(ConfigInvalid::new_err(
+ "factor must be a finite value greater than or equal to
1.0",
+ ));
+ }
retry = retry.with_factor(factor);
}
if jitter {
retry = retry.with_jitter();
}
if let Some(max_delay) = max_delay {
- retry = retry.with_max_delay(Duration::from_micros((max_delay *
1_000_000.0) as u64));
+ let max_delay = Duration::try_from_secs_f64(max_delay).map_err(|_|
{
+ ConfigInvalid::new_err("max_delay must be a finite,
non-negative number of seconds")
+ })?;
+ retry = retry.with_max_delay(max_delay);
}
if let Some(min_delay) = min_delay {
- retry = retry.with_min_delay(Duration::from_micros((min_delay *
1_000_000.0) as u64));
+ let min_delay = Duration::try_from_secs_f64(min_delay).map_err(|_|
{
+ ConfigInvalid::new_err("min_delay must be a finite,
non-negative number of seconds")
+ })?;
+ retry = retry.with_min_delay(min_delay);
}
let retry_layer = Self(retry);
diff --git a/bindings/python/tests/layers/test_retry_layer.py
b/bindings/python/tests/layers/test_retry_layer.py
new file mode 100644
index 000000000..58cd392c8
--- /dev/null
+++ b/bindings/python/tests/layers/test_retry_layer.py
@@ -0,0 +1,64 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+
+import opendal
+from opendal.exceptions import ConfigInvalid
+
+# Construction and input validation only. Retry behavior is not observable from
+# Python yet (no fault injection or attempt hook exposed); it is tested in the
+# Rust core and can be added here once a layer like ChaosLayer is bound.
+
+
[email protected](
+ "kwargs",
+ [
+ {"factor": 0.0},
+ {"factor": 0.5},
+ {"factor": float("nan")},
+ {"factor": float("inf")},
+ {"max_delay": -1.0},
+ {"max_delay": float("nan")},
+ {"max_delay": float("inf")},
+ {"max_delay": 1e30},
+ {"min_delay": -5.0},
+ {"min_delay": float("nan")},
+ {"min_delay": float("inf")},
+ {"min_delay": 1e30},
+ ],
+)
+def test_retry_layer_rejects_invalid_values(kwargs):
+ with pytest.raises(ConfigInvalid):
+ opendal.layers.RetryLayer(**kwargs)
+
+
[email protected](
+ "kwargs",
+ [
+ {},
+ {"factor": 1.0},
+ {"factor": 2.5},
+ {"max_delay": 0.0},
+ {"max_delay": 60.0},
+ {"min_delay": 0.0},
+ {"min_delay": 1.0},
+ {"max_times": 5, "factor": 2.0, "max_delay": 30.0, "min_delay": 1.0},
+ ],
+)
+def test_retry_layer_accepts_valid_values(kwargs):
+ assert isinstance(opendal.layers.RetryLayer(**kwargs),
opendal.layers.RetryLayer)
