Address review feedback: move defaults to Python, validate in Rust

Harry-kp · Harry-kp · commit f2308d6dfcd1 · 2026-02-28T00:26:08.000+05:30
- Move LMDB default values to Python side (lmdb_max_dbs=1024,
  lmdb_map_size=4GiB) so fields are never None, matching the pattern
  used by GlobalExecutionOptions.source_max_inflight_rows
- Make Rust EnvironmentSettings fields non-optional (u32/usize) with
  serde default functions for backward compatibility
- Add validation with client_bail! for invalid values (max_dbs &lt; 1,
  map_size == 0)
- Remove unused u32 import from std
diff --git a/python/cocoindex/_internal/setting.py b/python/cocoindex/_internal/setting.py
@@ -58,8 +58,8 @@ class Settings:
 
     db_path: os.PathLike[str] | None = None
     global_execution_options: GlobalExecutionOptions | None = None
-    lmdb_max_dbs: int | None = None
-    lmdb_map_size: int | None = None
+    lmdb_max_dbs: int = 1024
+    lmdb_map_size: int = 0x1_0000_0000  # 4 GiB
 
     @classmethod
     def from_env(cls, db_path: os.PathLike[str] | None = None) -> Self:
diff --git a/python/cocoindex/setting.py b/python/cocoindex/setting.py
@@ -46,8 +46,8 @@ class Settings:
 
     db_path: os.PathLike[str] | None = None
     global_execution_options: GlobalExecutionOptions | None = None
-    lmdb_max_dbs: int | None = None
-    lmdb_map_size: int | None = None
+    lmdb_max_dbs: int = 1024
+    lmdb_map_size: int = 0x1_0000_0000  # 4 GiB
 
     @classmethod
     def from_env(cls, db_path: os.PathLike[str] | None = None) -> Self:
diff --git a/rust/core/src/engine/environment.rs b/rust/core/src/engine/environment.rs
@@ -3,18 +3,25 @@ use crate::{
 };
 
 use serde::{Deserialize, Serialize};
-use std::{collections::BTreeSet, path::PathBuf, u32};
+use std::{collections::BTreeSet, path::PathBuf};
 
 const DEFAULT_MAX_DBS: u32 = 1024;
 const DEFAULT_LMDB_MAP_SIZE: usize = 0x1_0000_0000; // 4GiB
 
+fn default_max_dbs() -> u32 {
+    DEFAULT_MAX_DBS
+}
+fn default_lmdb_map_size() -> usize {
+    DEFAULT_LMDB_MAP_SIZE
+}
+
 #[derive(Clone, Serialize, Deserialize, Debug)]
 pub struct EnvironmentSettings {
     pub db_path: PathBuf,
-    #[serde(default)]
-    pub lmdb_max_dbs: Option<u32>,
-    #[serde(default)]
-    pub lmdb_map_size: Option<usize>,
+    #[serde(default = "default_max_dbs")]
+    pub lmdb_max_dbs: u32,
+    #[serde(default = "default_lmdb_map_size")]
+    pub lmdb_map_size: usize,
 }
 
 struct EnvironmentInner<Prof: EngineProfile> {
@@ -40,12 +47,16 @@ impl<Prof: EngineProfile> Environment<Prof> {
         std::fs::create_dir_all(&db_path)?;
         // Backward compatibility: migrate LMDB files from old layout into mdb/.
         Self::migrate_legacy_db_files(&settings.db_path, &db_path)?;
-        let max_dbs = settings.lmdb_max_dbs.unwrap_or(DEFAULT_MAX_DBS);
-        let map_size = settings.lmdb_map_size.unwrap_or(DEFAULT_LMDB_MAP_SIZE);
+        if settings.lmdb_max_dbs < 1 {
+            client_bail!("lmdb_max_dbs must be >= 1, got {}", settings.lmdb_max_dbs);
+        }
+        if settings.lmdb_map_size == 0 {
+            client_bail!("lmdb_map_size must be > 0, got {}", settings.lmdb_map_size);
+        }
         let db_env = unsafe {
             heed::EnvOpenOptions::new()
-                .max_dbs(max_dbs)
-                .map_size(map_size)
+                .max_dbs(settings.lmdb_max_dbs)
+                .map_size(settings.lmdb_map_size)
                 .open(db_path)
         }?;
         let cleared_count = db_env.clear_stale_readers()?;
