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/07 05:08:01 UTC
(opendal) branch main updated: Revert "fix(core): deserialize duration config values from strings" (#8034)
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 399cfbf3a4 Revert "fix(core): deserialize duration config values from strings" (#8034)
399cfbf3a4 is described below
commit 399cfbf3a4b8cc6ec0f4c3a8f74b322254c40581
Author: Erick Guan <29...@users.noreply.github.com>
AuthorDate: Fri Aug 7 13:07:54 2026 +0800
Revert "fix(core): deserialize duration config values from strings" (#8034)
Revert "fix(core): deserialize duration config values from strings (#7983)"
This reverts commit 188f0c0744991f8fb2dc703ade6fd272832743c4.
Exclusion for 0.58.2 release.
---
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, 21 insertions(+), 104 deletions(-)
diff --git a/core/core/src/raw/time.rs b/core/core/src/raw/time.rs
index 4ff95f4463..f1a3e3c4dc 100644
--- a/core/core/src/raw/time.rs
+++ b/core/core/src/raw/time.rs
@@ -18,12 +18,11 @@
//! 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};
@@ -255,6 +254,7 @@ 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,13 +262,7 @@ pub fn signed_to_duration(value: &str) -> Result<Duration> {
Error::new(ErrorKind::ConfigInvalid, "failed to parse duration").set_source(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| {
+ Duration::try_from(signed).map_err(|err| {
Error::new(
ErrorKind::ConfigInvalid,
"duration must not be negative or overflow",
@@ -310,9 +304,4 @@ 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 bd536df6ab..a900fb6036 100644
--- a/core/services/cloudflare-kv/src/backend.rs
+++ b/core/services/cloudflare-kv/src/backend.rs
@@ -37,7 +37,6 @@ use super::writer::CloudflareWriter;
#[derive(Default)]
pub struct CloudflareKvBuilder {
pub(super) config: CloudflareKvConfig,
- pub(super) default_ttl: Option<Duration>,
}
impl Debug for CloudflareKvBuilder {
@@ -77,7 +76,7 @@ impl CloudflareKvBuilder {
///
/// If set, we will specify `EX` for write operations.
pub fn default_ttl(mut self, ttl: Duration) -> Self {
- self.default_ttl = Some(ttl);
+ self.config.default_ttl = Some(ttl);
self
}
@@ -97,14 +96,6 @@ 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 => {
@@ -130,7 +121,7 @@ impl Builder for CloudflareKvBuilder {
};
// Validate default TTL is at least 60 seconds if specified
- if let Some(ttl) = default_ttl
+ if let Some(ttl) = self.config.default_ttl
&& ttl < Duration::from_secs(60)
{
return Err(Error::new(
@@ -152,7 +143,7 @@ impl Builder for CloudflareKvBuilder {
api_token,
account_id,
namespace_id,
- expiration_ttl: default_ttl,
+ expiration_ttl: self.config.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 7f5f2bae8f..cad44d7ea3 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<SignedDuration>,
+ pub default_ttl: Option<Duration>,
/// Root within this backend.
pub root: Option<String>,
@@ -94,10 +94,7 @@ impl opendal_core::Configurator for CloudflareKvConfig {
}
fn into_builder(self) -> Self::Builder {
- CloudflareKvBuilder {
- config: self,
- default_ttl: None,
- }
+ CloudflareKvBuilder { config: self }
}
}
@@ -128,12 +125,4 @@ 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 2a259892fe..7d6e4a2d85 100644
--- a/core/services/memcached/src/backend.rs
+++ b/core/services/memcached/src/backend.rs
@@ -34,7 +34,6 @@ use super::writer::MemcachedWriter;
#[derive(Debug, Default)]
pub struct MemcachedBuilder {
pub(super) config: MemcachedConfig,
- pub(super) default_ttl: Option<Duration>,
}
impl MemcachedBuilder {
@@ -75,7 +74,7 @@ impl MemcachedBuilder {
/// Set the default ttl for memcached services.
pub fn default_ttl(mut self, ttl: Duration) -> Self {
- self.default_ttl = Some(ttl);
+ self.config.default_ttl = Some(ttl);
self
}
@@ -98,14 +97,6 @@ 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)
@@ -180,7 +171,7 @@ impl Builder for MemcachedBuilder {
endpoint,
self.config.username,
self.config.password,
- default_ttl,
+ self.config.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 4cc2ad2b81..df8ab16b0c 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<SignedDuration>,
+ pub default_ttl: Option<Duration>,
/// The maximum number of connections allowed.
///
/// default is 10
@@ -81,10 +81,7 @@ impl Configurator for MemcachedConfig {
}
fn into_builder(self) -> Self::Builder {
- MemcachedBuilder {
- config: self,
- default_ttl: None,
- }
+ MemcachedBuilder { config: self }
}
}
@@ -104,12 +101,4 @@ 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 a86665159e..67aa3896d3 100644
--- a/core/services/redis/src/backend.rs
+++ b/core/services/redis/src/backend.rs
@@ -44,7 +44,6 @@ const DEFAULT_REDIS_PORT: u16 = 6379;
#[derive(Debug, Default)]
pub struct RedisBuilder {
pub(super) config: RedisConfig,
- pub(super) default_ttl: Option<Duration>,
}
impl RedisBuilder {
@@ -109,7 +108,7 @@ impl RedisBuilder {
///
/// If set, we will specify `EX` for write operations.
pub fn default_ttl(mut self, ttl: Duration) -> Self {
- self.default_ttl = Some(ttl);
+ self.config.default_ttl = Some(ttl);
self
}
@@ -145,14 +144,6 @@ 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
@@ -179,7 +170,7 @@ impl Builder for RedisBuilder {
endpoints,
None,
Some(client),
- default_ttl,
+ self.config.default_ttl,
self.config.connection_pool_max_size,
))
.with_normalized_root(root))
@@ -203,7 +194,7 @@ impl Builder for RedisBuilder {
endpoint,
Some(client),
None,
- default_ttl,
+ self.config.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 4f488dac8e..e617be4990 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<SignedDuration>,
+ pub default_ttl: Option<Duration>,
}
impl Debug for RedisConfig {
@@ -120,10 +120,7 @@ impl Configurator for RedisConfig {
}
fn into_builder(self) -> Self::Builder {
- RedisBuilder {
- config: self,
- default_ttl: None,
- }
+ RedisBuilder { config: self }
}
}
@@ -159,14 +156,6 @@ 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 df200daaa0..f46eaf697b 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` and `SignedDuration`
+ /// Mapping to rust's `Duration`
Duration,
/// Mapping to rust's `usize`
@@ -113,7 +113,7 @@ impl FromStr for ConfigType {
Ok(match s {
"bool" => ConfigType::Bool,
"String" => ConfigType::String,
- "Duration" | "SignedDuration" => ConfigType::Duration,
+ "Duration" => ConfigType::Duration,
"usize" => ConfigType::Usize,
"u64" => ConfigType::U64,
@@ -514,19 +514,6 @@ 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 963048f2d0..b866349ffc 100644
--- a/dev/src/generate/python.rs
+++ b/dev/src/generate/python.rs
@@ -264,7 +264,8 @@ 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 values cross the Python binding as strings, e.g. "5s".
+ // Duration is typed `str` (a humantime string, e.g. "5s"). See #7887:
+ // core cannot yet deserialize Duration config fields from strings.
ConfigType::Duration => "str".to_string(),
ConfigType::Usize
| ConfigType::U64