You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by er...@apache.org on 2026/08/21 03:40:36 UTC
(opendal) branch main updated: Reapply "fix(core): deserialize duration config values from strings" (#8123)
This is an automated email from the ASF dual-hosted git repository.
erickguan 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 dcbf53ea46 Reapply "fix(core): deserialize duration config values from strings" (#8123)
dcbf53ea46 is described below
commit dcbf53ea46fd816ff665e2fed2e14162115d9086
Author: Erick Guan <29...@users.noreply.github.com>
AuthorDate: Fri Aug 21 11:40:29 2026 +0800
Reapply "fix(core): deserialize duration config values from strings" (#8123)
---
core/core/src/raw/time.rs | 17 ++++++++++++++---
core/services/cloudflare-kv/src/backend.rs | 15 ++++++++++++---
core/services/cloudflare-kv/src/config.rs | 15 +++++++++++++--
core/services/memcached/src/backend.rs | 13 +++++++++++--
core/services/memcached/src/config.rs | 15 +++++++++++++--
core/services/redis/src/backend.rs | 15 ++++++++++++---
core/services/redis/src/config.rs | 15 +++++++++++++--
dev/src/generate/parser.rs | 17 +++++++++++++++--
dev/src/generate/python.rs | 3 +--
9 files changed, 104 insertions(+), 21 deletions(-)
diff --git a/core/core/src/raw/time.rs b/core/core/src/raw/time.rs
index f1a3e3c4dc..4ff95f4463 100644
--- a/core/core/src/raw/time.rs
+++ b/core/core/src/raw/time.rs
@@ -18,11 +18,12 @@
//! Time related utils.
use crate::*;
-use jiff::SignedDuration;
+
use std::fmt;
use std::ops::{Add, AddAssign, Sub, SubAssign};
use std::str::FromStr;
+pub use jiff::SignedDuration;
pub use std::time::{Duration, UNIX_EPOCH};
#[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))]
pub use std::time::{Instant, SystemTime};
@@ -254,7 +255,6 @@ impl SubAssign<Duration> for Timestamp {
}
}
-/// Convert an unsigned [`Duration`] into a jiff [`SignedDuration`].
/// Parse a duration encoded either as ISO-8601 (e.g. `PT5M`) or friendly (e.g. `5m`).
#[inline]
pub fn signed_to_duration(value: &str) -> Result<Duration> {
@@ -262,7 +262,13 @@ pub fn signed_to_duration(value: &str) -> Result<Duration> {
Error::new(ErrorKind::ConfigInvalid, "failed to parse duration").set_source(err)
})?;
- Duration::try_from(signed).map_err(|err| {
+ signed_duration_to_duration(signed)
+}
+
+/// Convert a jiff [`SignedDuration`] into an unsigned [`Duration`].
+#[inline]
+pub fn signed_duration_to_duration(value: SignedDuration) -> Result<Duration> {
+ Duration::try_from(value).map_err(|err| {
Error::new(
ErrorKind::ConfigInvalid,
"duration must not be negative or overflow",
@@ -304,4 +310,9 @@ mod tests {
let v = Timestamp::parse_rfc2822(s).unwrap();
assert_eq!("Sat, 29 Oct 1994 19:43:31 GMT", v.format_http_date());
}
+
+ #[test]
+ fn test_signed_duration_to_duration_rejects_negative_values() {
+ assert!(signed_duration_to_duration(SignedDuration::from_secs(-1)).is_err());
+ }
}
diff --git a/core/services/cloudflare-kv/src/backend.rs b/core/services/cloudflare-kv/src/backend.rs
index a900fb6036..bd536df6ab 100644
--- a/core/services/cloudflare-kv/src/backend.rs
+++ b/core/services/cloudflare-kv/src/backend.rs
@@ -37,6 +37,7 @@ use super::writer::CloudflareWriter;
#[derive(Default)]
pub struct CloudflareKvBuilder {
pub(super) config: CloudflareKvConfig,
+ pub(super) default_ttl: Option<Duration>,
}
impl Debug for CloudflareKvBuilder {
@@ -76,7 +77,7 @@ impl CloudflareKvBuilder {
///
/// If set, we will specify `EX` for write operations.
pub fn default_ttl(mut self, ttl: Duration) -> Self {
- self.config.default_ttl = Some(ttl);
+ self.default_ttl = Some(ttl);
self
}
@@ -96,6 +97,14 @@ impl Builder for CloudflareKvBuilder {
type Config = CloudflareKvConfig;
fn build(self) -> Result<impl Service> {
+ let default_ttl = match self.default_ttl {
+ Some(ttl) => Some(ttl),
+ None => self
+ .config
+ .default_ttl
+ .map(signed_duration_to_duration)
+ .transpose()?,
+ };
let api_token = match &self.config.api_token {
Some(api_token) => format_authorization_by_bearer(api_token)?,
None => {
@@ -121,7 +130,7 @@ impl Builder for CloudflareKvBuilder {
};
// Validate default TTL is at least 60 seconds if specified
- if let Some(ttl) = self.config.default_ttl
+ if let Some(ttl) = default_ttl
&& ttl < Duration::from_secs(60)
{
return Err(Error::new(
@@ -143,7 +152,7 @@ impl Builder for CloudflareKvBuilder {
api_token,
account_id,
namespace_id,
- expiration_ttl: self.config.default_ttl,
+ expiration_ttl: default_ttl,
info: ServiceInfo::new(CLOUDFLARE_KV_SCHEME, &root, ""),
capability: Capability {
create_dir: true,
diff --git a/core/services/cloudflare-kv/src/config.rs b/core/services/cloudflare-kv/src/config.rs
index cad44d7ea3..7f5f2bae8f 100644
--- a/core/services/cloudflare-kv/src/config.rs
+++ b/core/services/cloudflare-kv/src/config.rs
@@ -34,7 +34,7 @@ pub struct CloudflareKvConfig {
/// The namespace ID. Used as URI path parameter.
pub namespace_id: Option<String>,
/// The default ttl for write operations.
- pub default_ttl: Option<Duration>,
+ pub default_ttl: Option<SignedDuration>,
/// Root within this backend.
pub root: Option<String>,
@@ -94,7 +94,10 @@ impl opendal_core::Configurator for CloudflareKvConfig {
}
fn into_builder(self) -> Self::Builder {
- CloudflareKvBuilder { config: self }
+ CloudflareKvBuilder {
+ config: self,
+ default_ttl: None,
+ }
}
}
@@ -125,4 +128,12 @@ mod tests {
assert!(CloudflareKvConfig::from_uri(&uri).is_err());
}
+
+ #[test]
+ fn from_iter_parses_default_ttl() {
+ let cfg = CloudflareKvConfig::from_iter([("default_ttl".to_string(), "PT1M".to_string())])
+ .unwrap();
+
+ assert_eq!(cfg.default_ttl, Some(SignedDuration::from_mins(1)));
+ }
}
diff --git a/core/services/memcached/src/backend.rs b/core/services/memcached/src/backend.rs
index 7d6e4a2d85..2a259892fe 100644
--- a/core/services/memcached/src/backend.rs
+++ b/core/services/memcached/src/backend.rs
@@ -34,6 +34,7 @@ use super::writer::MemcachedWriter;
#[derive(Debug, Default)]
pub struct MemcachedBuilder {
pub(super) config: MemcachedConfig,
+ pub(super) default_ttl: Option<Duration>,
}
impl MemcachedBuilder {
@@ -74,7 +75,7 @@ impl MemcachedBuilder {
/// Set the default ttl for memcached services.
pub fn default_ttl(mut self, ttl: Duration) -> Self {
- self.config.default_ttl = Some(ttl);
+ self.default_ttl = Some(ttl);
self
}
@@ -97,6 +98,14 @@ impl Builder for MemcachedBuilder {
type Config = MemcachedConfig;
fn build(self) -> Result<impl Service> {
+ let default_ttl = match self.default_ttl {
+ Some(ttl) => Some(ttl),
+ None => self
+ .config
+ .default_ttl
+ .map(signed_duration_to_duration)
+ .transpose()?,
+ };
let endpoint_raw = self.config.endpoint.clone().ok_or_else(|| {
Error::new(ErrorKind::ConfigInvalid, "endpoint is empty")
.with_context("service", MEMCACHED_SCHEME)
@@ -171,7 +180,7 @@ impl Builder for MemcachedBuilder {
endpoint,
self.config.username,
self.config.password,
- self.config.default_ttl,
+ default_ttl,
self.config.connection_pool_max_size,
))
.with_normalized_root(root))
diff --git a/core/services/memcached/src/config.rs b/core/services/memcached/src/config.rs
index df8ab16b0c..4cc2ad2b81 100644
--- a/core/services/memcached/src/config.rs
+++ b/core/services/memcached/src/config.rs
@@ -44,7 +44,7 @@ pub struct MemcachedConfig {
/// Memcached password, optional.
pub password: Option<String>,
/// The default ttl for put operations.
- pub default_ttl: Option<Duration>,
+ pub default_ttl: Option<SignedDuration>,
/// The maximum number of connections allowed.
///
/// default is 10
@@ -81,7 +81,10 @@ impl Configurator for MemcachedConfig {
}
fn into_builder(self) -> Self::Builder {
- MemcachedBuilder { config: self }
+ MemcachedBuilder {
+ config: self,
+ default_ttl: None,
+ }
}
}
@@ -101,4 +104,12 @@ mod tests {
assert_eq!(cfg.root.as_deref(), Some("app/session"));
Ok(())
}
+
+ #[test]
+ fn from_iter_parses_default_ttl() -> Result<()> {
+ let cfg = MemcachedConfig::from_iter([("default_ttl".to_string(), "1500ms".to_string())])?;
+
+ assert_eq!(cfg.default_ttl, Some(SignedDuration::from_millis(1500)));
+ Ok(())
+ }
}
diff --git a/core/services/redis/src/backend.rs b/core/services/redis/src/backend.rs
index 67aa3896d3..a86665159e 100644
--- a/core/services/redis/src/backend.rs
+++ b/core/services/redis/src/backend.rs
@@ -44,6 +44,7 @@ const DEFAULT_REDIS_PORT: u16 = 6379;
#[derive(Debug, Default)]
pub struct RedisBuilder {
pub(super) config: RedisConfig,
+ pub(super) default_ttl: Option<Duration>,
}
impl RedisBuilder {
@@ -108,7 +109,7 @@ impl RedisBuilder {
///
/// If set, we will specify `EX` for write operations.
pub fn default_ttl(mut self, ttl: Duration) -> Self {
- self.config.default_ttl = Some(ttl);
+ self.default_ttl = Some(ttl);
self
}
@@ -144,6 +145,14 @@ impl Builder for RedisBuilder {
type Config = RedisConfig;
fn build(self) -> Result<impl Service> {
+ let default_ttl = match self.default_ttl {
+ Some(ttl) => Some(ttl),
+ None => self
+ .config
+ .default_ttl
+ .map(signed_duration_to_duration)
+ .transpose()?,
+ };
let root = normalize_root(
self.config
.root
@@ -170,7 +179,7 @@ impl Builder for RedisBuilder {
endpoints,
None,
Some(client),
- self.config.default_ttl,
+ default_ttl,
self.config.connection_pool_max_size,
))
.with_normalized_root(root))
@@ -194,7 +203,7 @@ impl Builder for RedisBuilder {
endpoint,
Some(client),
None,
- self.config.default_ttl,
+ default_ttl,
self.config.connection_pool_max_size,
))
.with_normalized_root(root))
diff --git a/core/services/redis/src/config.rs b/core/services/redis/src/config.rs
index e617be4990..4f488dac8e 100644
--- a/core/services/redis/src/config.rs
+++ b/core/services/redis/src/config.rs
@@ -59,7 +59,7 @@ pub struct RedisConfig {
/// default is db 0
pub db: i64,
/// The default ttl for put operations.
- pub default_ttl: Option<Duration>,
+ pub default_ttl: Option<SignedDuration>,
}
impl Debug for RedisConfig {
@@ -120,7 +120,10 @@ impl Configurator for RedisConfig {
}
fn into_builder(self) -> Self::Builder {
- RedisBuilder { config: self }
+ RedisBuilder {
+ config: self,
+ default_ttl: None,
+ }
}
}
@@ -156,6 +159,14 @@ mod tests {
Ok(())
}
+ #[test]
+ fn from_iter_parses_default_ttl() -> Result<()> {
+ let cfg = RedisConfig::from_iter([("default_ttl".to_string(), "5s".to_string())])?;
+
+ assert_eq!(cfg.default_ttl, Some(SignedDuration::from_secs(5)));
+ Ok(())
+ }
+
#[test]
fn test_redis_builder_interface() {
// Test that RedisBuilder still works with the new implementation
diff --git a/dev/src/generate/parser.rs b/dev/src/generate/parser.rs
index f46eaf697b..df200daaa0 100644
--- a/dev/src/generate/parser.rs
+++ b/dev/src/generate/parser.rs
@@ -83,7 +83,7 @@ pub enum ConfigType {
Bool,
/// Mapping to rust's `String`
String,
- /// Mapping to rust's `Duration`
+ /// Mapping to rust's `Duration` and `SignedDuration`
Duration,
/// Mapping to rust's `usize`
@@ -113,7 +113,7 @@ impl FromStr for ConfigType {
Ok(match s {
"bool" => ConfigType::Bool,
"String" => ConfigType::String,
- "Duration" => ConfigType::Duration,
+ "Duration" | "SignedDuration" => ConfigType::Duration,
"usize" => ConfigType::Usize,
"u64" => ConfigType::U64,
@@ -514,6 +514,19 @@ mod tests {
example: None,
},
),
+ (
+ "default_ttl: Option<SignedDuration>",
+ Config {
+ name: "default_ttl".to_string(),
+ value: ConfigType::Duration,
+ optional: true,
+ deprecated: None,
+ comments: "".to_string(),
+ group: None,
+ default_value: None,
+ example: None,
+ },
+ ),
];
for (input, expected) in cases {
diff --git a/dev/src/generate/python.rs b/dev/src/generate/python.rs
index b866349ffc..963048f2d0 100644
--- a/dev/src/generate/python.rs
+++ b/dev/src/generate/python.rs
@@ -264,8 +264,7 @@ fn is_path_like_field(name: &str) -> bool {
fn make_config_field_type(field: ViaDeserialize<Config>) -> Result<String, minijinja::Error> {
Ok(match field.value {
ConfigType::Bool => "bool".to_string(),
- // Duration is typed `str` (a humantime string, e.g. "5s"). See #7887:
- // core cannot yet deserialize Duration config fields from strings.
+ // Duration values cross the Python binding as strings, e.g. "5s".
ConfigType::Duration => "str".to_string(),
ConfigType::Usize
| ConfigType::U64