diff --git a/docs/CLI.md b/docs/CLI.md index 646043a0..c6f92505 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -42,7 +42,7 @@ modelexpress-cli [OPTIONS] - `-f, --format `: Output format: `human`, `json`, `json-pretty` (default: human) - `-v, -vv, -vvv`: Verbose mode (info, debug, trace) - `-q, --quiet`: Quiet mode (suppress all output except errors) -- `--cache-path `: Model storage path override +- `--cache-directory `: Cache directory override - `--no-shared-storage`: Disable shared storage mode (will transfer files from server to client) - `--transfer-chunk-size `: Chunk size in bytes for file transfer when shared storage is disabled (default: 32768) - `-h, --help`: Print help information @@ -50,7 +50,7 @@ modelexpress-cli [OPTIONS] **Environment Variables:** - `MODEL_EXPRESS_ENDPOINT`: Set the default server endpoint -- `MODEL_EXPRESS_CACHE_DIRECTORY`: Set the default model storage path +- `MODEL_EXPRESS_CACHE_DIRECTORY`: Set the default cache directory - `MODEL_EXPRESS_NO_SHARED_STORAGE`: Disable shared storage mode (set to 'true' to enable file transfers) - `MODEL_EXPRESS_TRANSFER_CHUNK_SIZE`: Set the chunk size in bytes for file transfers @@ -102,9 +102,8 @@ modelexpress-cli --no-shared-storage --transfer-chunk-size 65536 \ modelexpress-cli model init # Initialize with custom settings -modelexpress-cli model init \ - --storage-path /path/to/your/models \ - --server-endpoint http://localhost:8001 +modelexpress-cli --cache-directory /path/to/your/models \ + model init --server-endpoint http://localhost:8001 # List downloaded models modelexpress-cli model list @@ -233,8 +232,8 @@ modelexpress-cli --endpoint https://my-server.com:8001 \ modelexpress-cli -vv model download microsoft/DialoGPT-small \ --strategy server-only -# Download model with custom storage path -modelexpress-cli --cache-path /custom/storage/path \ +# Download model with custom cache directory +modelexpress-cli --cache-directory /custom/cache/path \ model download google-t5/t5-small # Send API request with file payload and JSON output @@ -365,10 +364,10 @@ Set default values using environment variables: # Set default server endpoint export MODEL_EXPRESS_ENDPOINT="https://my-server.com:8001" -# Set default cache path +# Set default cache directory export MODEL_EXPRESS_CACHE_DIRECTORY="/path/to/storage" -# Use the CLI without specifying endpoint or storage path +# Use the CLI without specifying endpoint or cache directory modelexpress-cli health modelexpress-cli model status ``` diff --git a/examples/aggregated_k8s/agg.yaml b/examples/aggregated_k8s/agg.yaml index f8c6d651..80f71256 100644 --- a/examples/aggregated_k8s/agg.yaml +++ b/examples/aggregated_k8s/agg.yaml @@ -89,7 +89,7 @@ spec: mkdir -p $MODEL_CACHE_PATH mkdir -p /model/.model-express cat > /model/.model-express/config.yaml << EOF - local_path: $MODEL_CACHE_PATH + directory: $MODEL_CACHE_PATH server_endpoint: http://localhost:8000 timeout_secs: null EOF diff --git a/modelexpress-cli-completion.bash b/modelexpress-cli-completion.bash index 0d146a1e..8da10ad4 100644 --- a/modelexpress-cli-completion.bash +++ b/modelexpress-cli-completion.bash @@ -12,7 +12,7 @@ _model_express_cli_completions() { # Main commands local commands="health model api help" - local global_opts="--endpoint --timeout --format --verbose --quiet --cache-path --help --version" + local global_opts="--endpoint --timeout --format --verbose --quiet --cache-directory --help --version" local formats="human json json-pretty" case "${COMP_CWORD}" in @@ -42,7 +42,7 @@ _model_express_cli_completions() { COMPREPLY=($(compgen -f -- "$cur")) return 0 ;; - --cache-path) + --cache-directory) COMPREPLY=($(compgen -d -- "$cur")) return 0 ;; @@ -90,15 +90,12 @@ _model_express_cli_completions() { esac elif [[ "${words[i+1]}" == "init" ]]; then case "${prev}" in - --storage-path) - COMPREPLY=($(compgen -d -- "$cur")) - ;; --server-endpoint) COMPREPLY=($(compgen -W "http://localhost:8001 https://" -- "$cur")) ;; *) if [[ "$cur" == -* ]]; then - COMPREPLY=($(compgen -W "--storage-path --server-endpoint --help" -- "$cur")) + COMPREPLY=($(compgen -W "--server-endpoint --help" -- "$cur")) fi ;; esac diff --git a/modelexpress_client/src/bin/cli.rs b/modelexpress_client/src/bin/cli.rs index fc285874..3e69a8a7 100644 --- a/modelexpress_client/src/bin/cli.rs +++ b/modelexpress_client/src/bin/cli.rs @@ -8,7 +8,7 @@ //! This CLI uses a layered argument structure: //! //! - **`ClientArgs`** (in `modelexpress_common/src/client_config.rs`): -//! Shared arguments like `--endpoint`, `--timeout`, `--cache-path`, etc. +//! Shared arguments like `--endpoint`, `--timeout`, `--cache-directory`, etc. //! These support environment variables (e.g., `MODEL_EXPRESS_ENDPOINT`). //! Add new shared arguments there. //! @@ -71,7 +71,7 @@ async fn main() { debug!("Executing model command"); handle_model_command( command, - cli.client_args.cache_path.clone(), + cli.client_args.cache_directory.clone(), config, &cli.format, ) @@ -152,17 +152,17 @@ mod tests { } #[test] - fn test_cli_with_cache_path() { + fn test_cli_with_cache_directory() { let cli = Cli::try_parse_from([ "modelexpress-cli", - "--cache-path", + "--cache-directory", "/custom/cache/path", "health", ]) - .expect("Failed to parse CLI with cache path"); + .expect("Failed to parse CLI with cache directory"); assert_eq!( - cli.client_args.cache_path, + cli.client_args.cache_directory, Some(std::path::PathBuf::from("/custom/cache/path")) ); } diff --git a/modelexpress_client/src/bin/modules/args.rs b/modelexpress_client/src/bin/modules/args.rs index 19176c49..07fbe7ce 100644 --- a/modelexpress_client/src/bin/modules/args.rs +++ b/modelexpress_client/src/bin/modules/args.rs @@ -3,7 +3,6 @@ use clap::{Parser, Subcommand, ValueEnum}; use modelexpress_client::{ClientArgs, ModelProvider}; -use std::path::PathBuf; /// CLI argument structure for the modelexpress-cli binary. /// @@ -85,10 +84,6 @@ pub enum ModelCommands { /// Initialize model storage configuration Init { - /// Storage path for models - #[arg(long, value_name = "PATH")] - storage_path: Option, - /// Server endpoint #[arg(long, value_name = "ENDPOINT")] server_endpoint: Option, diff --git a/modelexpress_client/src/bin/modules/handlers.rs b/modelexpress_client/src/bin/modules/handlers.rs index 9084f8b1..c26030ff 100644 --- a/modelexpress_client/src/bin/modules/handlers.rs +++ b/modelexpress_client/src/bin/modules/handlers.rs @@ -51,7 +51,7 @@ pub async fn handle_health_command( /// Handle model commands (unified model management) pub async fn handle_model_command( command: ModelCommands, - storage_path_override: Option, + cache_dir_override: Option, server_config: ClientConfig, format: &OutputFormat, ) -> Result<(), Box> { @@ -62,7 +62,7 @@ pub async fn handle_model_command( strategy, } => { download_model( - storage_path_override, + cache_dir_override, model_name, provider, strategy, @@ -71,31 +71,26 @@ pub async fn handle_model_command( ) .await } - ModelCommands::Init { - storage_path, - server_endpoint, - } => init_model_storage(storage_path, server_endpoint, format).await, - ModelCommands::List { detailed } => { - list_models(storage_path_override, detailed, format).await + ModelCommands::Init { server_endpoint } => { + init_model_storage(cache_dir_override, server_endpoint, format).await } - ModelCommands::Status => show_model_status(storage_path_override, format).await, + ModelCommands::List { detailed } => list_models(cache_dir_override, detailed, format).await, + ModelCommands::Status => show_model_status(cache_dir_override, format).await, ModelCommands::Clear { model_name } => { - clear_model(storage_path_override, &model_name, format).await - } - ModelCommands::ClearAll { yes } => { - clear_all_models(storage_path_override, yes, format).await + clear_model(cache_dir_override, &model_name, format).await } + ModelCommands::ClearAll { yes } => clear_all_models(cache_dir_override, yes, format).await, ModelCommands::Validate { model_name } => { - validate_models(storage_path_override, model_name, format).await + validate_models(cache_dir_override, model_name, format).await } ModelCommands::Stats { detailed } => { - show_model_stats(storage_path_override, detailed, format).await + show_model_stats(cache_dir_override, detailed, format).await } } } async fn download_model( - storage_path_override: Option, + cache_dir_override: Option, model_name: String, provider: CliModelProvider, strategy: DownloadStrategy, @@ -120,7 +115,7 @@ async fn download_model( info!("Downloading model: {}", model_name); // Get cache config if available, applying settings from ClientConfig - let cache_config = if let Some(path) = storage_path_override { + let cache_config = if let Some(ref path) = cache_dir_override { Some(CacheConfig::from_path(path)?) } else { CacheConfig::discover().ok() @@ -165,7 +160,8 @@ async fn download_model( } DownloadStrategy::Direct => { debug!("Using direct download strategy"); - Client::download_model_directly(model_name.clone(), provider, false).await + Client::download_model_directly(model_name.clone(), provider, false, cache_dir_override) + .await } }; @@ -260,11 +256,11 @@ pub async fn handle_api_send( } async fn init_model_storage( - storage_path: Option, + cache_dir_override: Option, server_endpoint: Option, format: &OutputFormat, ) -> Result<(), Box> { - let config = if let Some(path) = storage_path { + let config = if let Some(path) = cache_dir_override { CacheConfig::from_path(path)? } else { // Use default configuration instead of prompting @@ -285,13 +281,13 @@ async fn init_model_storage( println!("{}", "ModelExpress Storage Configuration".green().bold()); println!("{}", "===================================".green().bold()); println!("Configuration saved successfully!"); - println!("Storage path: {:?}", config.local_path); + println!("Cache directory: {:?}", config.directory); println!("Server endpoint: {}", config.server_endpoint); } _ => { let output = serde_json::json!({ "success": true, - "storage_path": config.local_path, + "cache_directory": config.directory, "server_endpoint": config.server_endpoint, }); print_output(&output, format); @@ -302,11 +298,11 @@ async fn init_model_storage( } async fn list_models( - storage_path_override: Option, + cache_dir_override: Option, detailed: bool, format: &OutputFormat, ) -> Result<(), Box> { - let storage_config = get_storage_config(storage_path_override)?; + let storage_config = get_storage_config(cache_dir_override)?; let stats = storage_config.get_cache_stats()?; match format { @@ -370,13 +366,13 @@ async fn list_models( } async fn show_model_status( - storage_path_override: Option, + cache_dir_override: Option, format: &OutputFormat, ) -> Result<(), Box> { - let storage_config = get_storage_config(storage_path_override)?; + let storage_config = get_storage_config(cache_dir_override)?; let stats = storage_config.get_cache_stats()?; - let storage_accessible = storage_config.local_path.exists(); + let storage_accessible = storage_config.directory.exists(); let server_available = Client::new(ClientConfig::for_testing(&storage_config.server_endpoint)) .await .is_ok(); @@ -385,7 +381,7 @@ async fn show_model_status( OutputFormat::Human => { println!("{}", "Model Storage Status".green().bold()); println!("{}", "====================".green().bold()); - println!("Storage path: {:?}", storage_config.local_path); + println!("Cache directory: {:?}", storage_config.directory); println!("Server endpoint: {}", storage_config.server_endpoint); println!("Total models: {}", stats.total_models); println!("Total size: {}", stats.format_total_size()); @@ -406,7 +402,7 @@ async fn show_model_status( } _ => { let output = serde_json::json!({ - "storage_path": storage_config.local_path, + "cache_directory": storage_config.directory, "server_endpoint": storage_config.server_endpoint, "total_models": stats.total_models, "total_size": stats.format_total_size(), @@ -421,11 +417,11 @@ async fn show_model_status( } async fn clear_model( - storage_path_override: Option, + cache_dir_override: Option, model_name: &str, format: &OutputFormat, ) -> Result<(), Box> { - let storage_config = get_storage_config(storage_path_override)?; + let storage_config = get_storage_config(cache_dir_override)?; storage_config.clear_model(model_name)?; @@ -447,11 +443,11 @@ async fn clear_model( } async fn clear_all_models( - storage_path_override: Option, + cache_dir_override: Option, yes: bool, format: &OutputFormat, ) -> Result<(), Box> { - let storage_config = get_storage_config(storage_path_override)?; + let storage_config = get_storage_config(cache_dir_override)?; if !yes && matches!(format, OutputFormat::Human) { print!("Are you sure you want to clear all models from storage? [y/N]: "); @@ -485,15 +481,15 @@ async fn clear_all_models( } async fn validate_models( - storage_path_override: Option, + cache_dir_override: Option, model_name: Option, format: &OutputFormat, ) -> Result<(), Box> { - let storage_config = get_storage_config(storage_path_override)?; + let storage_config = get_storage_config(cache_dir_override)?; if let Some(name) = model_name { // Validate specific model - let model_path = storage_config.local_path.join(&name); + let model_path = storage_config.directory.join(&name); let exists = model_path.exists(); match format { @@ -558,11 +554,11 @@ async fn validate_models( } async fn show_model_stats( - storage_path_override: Option, + cache_dir_override: Option, detailed: bool, format: &OutputFormat, ) -> Result<(), Box> { - let storage_config = get_storage_config(storage_path_override)?; + let storage_config = get_storage_config(cache_dir_override)?; let stats = storage_config.get_cache_stats()?; match format { @@ -620,10 +616,10 @@ async fn show_model_stats( } fn get_storage_config( - storage_path_override: Option, + cache_dir_override: Option, ) -> Result> { // If storage path is provided via CLI, use it - if let Some(path) = storage_path_override { + if let Some(path) = cache_dir_override { return Ok(CacheConfig::from_path(path)?); } diff --git a/modelexpress_client/src/bin/test_client.rs b/modelexpress_client/src/bin/test_client.rs index 896b3e5d..dad03de3 100644 --- a/modelexpress_client/src/bin/test_client.rs +++ b/modelexpress_client/src/bin/test_client.rs @@ -181,7 +181,8 @@ async fn run_fallback_test(model_name: &str) -> Result<(), Box { info!("Model downloaded directly in {:?}", start_direct.elapsed()); } diff --git a/modelexpress_client/src/lib.rs b/modelexpress_client/src/lib.rs index f11dc5b9..816e9245 100644 --- a/modelexpress_client/src/lib.rs +++ b/modelexpress_client/src/lib.rs @@ -130,13 +130,13 @@ impl Client { } // Use client's cache config if available if let Some(cache_config) = &self.cache_config { - return cache_config.local_path.clone(); + return cache_config.directory.clone(); } // Fall back to discovery CacheConfig::discover() - .map(|config| config.local_path) - .unwrap_or_else(|_| CacheConfig::default().local_path) + .map(|config| config.directory) + .unwrap_or_else(|_| CacheConfig::default().directory) } pub async fn get_model_path(&self, model_name: &str) -> anyhow::Result { @@ -191,7 +191,7 @@ impl Client { "Server unavailable, pre-loading model {} directly. Error: {}", model_name, e ); - Self::download_model_directly(model_name, provider, ignore_weights).await + Self::download_model_directly(model_name, provider, ignore_weights, None).await } } } @@ -297,7 +297,7 @@ impl Client { ); // Fallback to direct download - let cache_dir = CacheConfig::discover().ok().map(|config| config.local_path); + let cache_dir = CacheConfig::discover().ok().map(|config| config.directory); match download::download_model(&model_name, provider, cache_dir, ignore_weights).await { Ok(_) => { info!( @@ -332,7 +332,7 @@ impl Client { })?; let chunk_size = cache_config.transfer_chunk_size as u32; - let local_cache_path = cache_config.local_path.clone(); + let local_cache_path = cache_config.directory.clone(); info!( "Streaming model {} files to {:?} with chunk size {} bytes", @@ -629,7 +629,7 @@ impl Client { Err(e) => { // If we can't even connect to the server, go straight to direct download info!("Cannot connect to server ({}), downloading directly...", e); - Client::download_model_directly(&model_name, provider, ignore_weights).await + Client::download_model_directly(&model_name, provider, ignore_weights, None).await } } } @@ -640,6 +640,7 @@ impl Client { model_name: impl Into, provider: ModelProvider, ignore_weights: bool, + cache_dir: Option, ) -> CommonResult<()> { let model_name = model_name.into(); info!( @@ -647,8 +648,9 @@ impl Client { model_name, provider ); - // Try to get cache configuration, but don't fail if not available - let cache_dir = CacheConfig::discover().ok().map(|config| config.local_path); + // Use provided cache dir, or try to discover one + let cache_dir = + cache_dir.or_else(|| CacheConfig::discover().ok().map(|config| config.directory)); download::download_model(&model_name, provider, cache_dir, ignore_weights) .await @@ -707,7 +709,7 @@ mod tests { fn test_cache_config_endpoint_override() { // Test that cache config can be created with a specific endpoint let mut cache_config = CacheConfig { - local_path: std::path::PathBuf::from("/test/path"), + directory: std::path::PathBuf::from("/test/path"), server_endpoint: "http://original-endpoint:1234".to_string(), timeout_secs: None, shared_storage: true, @@ -742,6 +744,7 @@ mod tests { "definitely-not-a-real-model-name-12345", ModelProvider::HuggingFace, false, + None, ) .await; diff --git a/modelexpress_common/src/cache.rs b/modelexpress_common/src/cache.rs index 0cb3012d..805be52f 100644 --- a/modelexpress_common/src/cache.rs +++ b/modelexpress_common/src/cache.rs @@ -12,8 +12,9 @@ use tracing::{debug, info, warn}; /// Configuration for model cache management #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CacheConfig { - /// Local path where models are cached - pub local_path: PathBuf, + /// Directory where models are cached + #[serde(alias = "local_path")] + pub directory: PathBuf, /// Server endpoint for model downloads pub server_endpoint: String, /// Timeout for cache operations @@ -39,7 +40,7 @@ impl Default for CacheConfig { fn default() -> Self { let home = Utils::get_home_dir().unwrap_or_else(|_| ".".to_string()); Self { - local_path: PathBuf::from(home).join(constants::DEFAULT_CACHE_PATH), + directory: PathBuf::from(home).join(constants::DEFAULT_CACHE_PATH), server_endpoint: format!("http://localhost:{}", constants::DEFAULT_GRPC_PORT), timeout_secs: None, shared_storage: constants::DEFAULT_SHARED_STORAGE, @@ -52,16 +53,10 @@ impl CacheConfig { /// Discover cache configuration pub fn discover() -> Result { // Priority order: - // 1. Command line argument (--cache-path) - // 2. Environment variable (MODEL_EXPRESS_CACHE_DIRECTORY) - // 3. Config file (~/.model-express/config.yaml) - // 4. Auto-detection (common paths) - // 5. Default fallback - - // Try command line args first - if let Some(path) = Self::get_cache_path_from_args() { - return Self::from_path(path); - } + // 1. Environment variable (MODEL_EXPRESS_CACHE_DIRECTORY) + // 2. Config file (~/.model-express/config.yaml) + // 3. Auto-detection (common paths) + // 4. Default fallback // Try environment variable if let Ok(path) = env::var("MODEL_EXPRESS_CACHE_DIRECTORY") { @@ -84,13 +79,13 @@ impl CacheConfig { } /// Create a cache configuration with explicit parameters - pub fn new(local_path: PathBuf, server_endpoint: Option) -> Result { + pub fn new(directory: PathBuf, server_endpoint: Option) -> Result { // Ensure the directory exists - fs::create_dir_all(&local_path) - .with_context(|| format!("Failed to create cache directory: {local_path:?}"))?; + fs::create_dir_all(&directory) + .with_context(|| format!("Failed to create cache directory: {directory:?}"))?; Ok(Self { - local_path, + directory, server_endpoint: server_endpoint.unwrap_or_else(Self::get_default_server_endpoint), timeout_secs: None, shared_storage: constants::DEFAULT_SHARED_STORAGE, @@ -100,14 +95,14 @@ impl CacheConfig { /// Create config from a specific path pub fn from_path>(path: P) -> Result { - let local_path = path.as_ref().to_path_buf(); + let directory = path.as_ref().to_path_buf(); // Ensure the directory exists - fs::create_dir_all(&local_path) - .with_context(|| format!("Failed to create cache directory: {local_path:?}"))?; + fs::create_dir_all(&directory) + .with_context(|| format!("Failed to create cache directory: {directory:?}"))?; Ok(Self { - local_path, + directory, server_endpoint: Self::get_default_server_endpoint(), timeout_secs: None, shared_storage: constants::DEFAULT_SHARED_STORAGE, @@ -165,7 +160,7 @@ impl CacheConfig { for path in common_paths { if path.exists() && path.is_dir() { return Ok(Self { - local_path: path, + directory: path, server_endpoint: Self::get_default_server_endpoint(), timeout_secs: None, shared_storage: constants::DEFAULT_SHARED_STORAGE, @@ -186,21 +181,6 @@ impl CacheConfig { Err(anyhow::anyhow!("Server not available for cache discovery")) } - /// Get cache path from command line arguments - fn get_cache_path_from_args() -> Option { - let args: Vec = env::args().collect(); - - for (i, arg) in args.iter().enumerate() { - if arg == "--cache-path" - && let Some(next_arg) = args.get(i.saturating_add(1)) - { - return Some(next_arg.clone()); - } - } - - None - } - /// Get default server endpoint fn get_default_server_endpoint() -> String { env::var("MODEL_EXPRESS_SERVER_ENDPOINT") @@ -242,11 +222,11 @@ impl CacheConfig { models: Vec::new(), }; - if !self.local_path.exists() { + if !self.directory.exists() { return Ok(stats); } - for entry in fs::read_dir(&self.local_path)? { + for entry in fs::read_dir(&self.directory)? { let entry = entry?; let path = entry.path(); @@ -294,7 +274,7 @@ impl CacheConfig { /// Clear specific model from cache pub fn clear_model(&self, model_name: &str) -> Result<()> { - let model_path = self.local_path.join(model_name); + let model_path = self.directory.join(model_name); if model_path.exists() { fs::remove_dir_all(&model_path) @@ -309,12 +289,12 @@ impl CacheConfig { /// Clear entire cache pub fn clear_all(&self) -> Result<()> { - if self.local_path.exists() { - for entry in fs::read_dir(&self.local_path) - .with_context(|| format!("Failed to read cache directory: {:?}", self.local_path))? + if self.directory.exists() { + for entry in fs::read_dir(&self.directory) + .with_context(|| format!("Failed to read cache directory: {:?}", self.directory))? { let entry = entry - .with_context(|| format!("Failed to read entry in: {:?}", self.local_path))?; + .with_context(|| format!("Failed to read entry in: {:?}", self.directory))?; let path = entry.path(); if path.is_dir() { fs::remove_dir_all(&path) @@ -388,7 +368,7 @@ mod tests { let config = CacheConfig::from_path(temp_dir.path()).expect("Failed to create config from path"); - assert_eq!(config.local_path, temp_dir.path()); + assert_eq!(config.directory, temp_dir.path()); } #[test] @@ -396,7 +376,7 @@ mod tests { fn test_cache_config_save_and_load() { let temp_dir = TempDir::new().expect("Failed to create temp directory"); let original_config = CacheConfig { - local_path: temp_dir.path().join("cache"), + directory: temp_dir.path().join("cache"), server_endpoint: "http://localhost:8001".to_string(), timeout_secs: Some(30), shared_storage: false, @@ -411,7 +391,7 @@ mod tests { // Load config let loaded_config = CacheConfig::from_config_file().expect("Failed to load config"); - assert_eq!(loaded_config.local_path, original_config.local_path); + assert_eq!(loaded_config.directory, original_config.directory); assert_eq!( loaded_config.server_endpoint, original_config.server_endpoint @@ -454,7 +434,7 @@ mod tests { let home = Utils::get_home_dir().unwrap_or_else(|_| ".".to_string()); assert_eq!( - config.local_path, + config.directory, PathBuf::from(&home).join(constants::DEFAULT_CACHE_PATH) ); assert_eq!( @@ -551,7 +531,7 @@ mod tests { fs::write(cache_path.join("test_file.txt"), "test").expect("Failed to write file"); let config = CacheConfig { - local_path: cache_path.clone(), + directory: cache_path.clone(), server_endpoint: "http://localhost:8001".to_string(), timeout_secs: None, shared_storage: false, @@ -579,7 +559,7 @@ mod tests { let cache_path = temp_dir.path().join("nonexistent_cache"); let config = CacheConfig { - local_path: cache_path.clone(), + directory: cache_path.clone(), server_endpoint: "http://localhost:8001".to_string(), timeout_secs: None, shared_storage: false, @@ -607,7 +587,7 @@ mod tests { fs::write(deep_path.join("deep_file.txt"), "deep").expect("Failed to write file"); let config = CacheConfig { - local_path: cache_path.clone(), + directory: cache_path.clone(), server_endpoint: "http://localhost:8001".to_string(), timeout_secs: None, shared_storage: false, diff --git a/modelexpress_common/src/client_config.rs b/modelexpress_common/src/client_config.rs index 20d30d1c..5294c7e8 100644 --- a/modelexpress_common/src/client_config.rs +++ b/modelexpress_common/src/client_config.rs @@ -46,9 +46,9 @@ pub struct ClientArgs { #[arg(short, long, env = "MODEL_EXPRESS_TIMEOUT")] pub timeout: Option, - /// Cache path override + /// Cache directory override #[arg(long, env = "MODEL_EXPRESS_CACHE_DIRECTORY")] - pub cache_path: Option, + pub cache_directory: Option, /// Log level (no short flag to avoid conflict with CLI's -v/--verbose) #[arg(long, env = "MODEL_EXPRESS_LOG_LEVEL", value_enum)] @@ -144,8 +144,8 @@ impl ClientConfig { } // Cache settings - if let Some(cache_path) = args.cache_path { - config.cache.local_path = cache_path; + if let Some(cache_directory) = args.cache_directory { + config.cache.directory = cache_directory; } if args.no_shared_storage { @@ -196,12 +196,12 @@ impl ClientConfig { } // Validate cache path exists or can be created - if !self.cache.local_path.exists() - && let Err(e) = std::fs::create_dir_all(&self.cache.local_path) + if !self.cache.directory.exists() + && let Err(e) = std::fs::create_dir_all(&self.cache.directory) { return Err(ConfigError::Message(format!( "Cannot create cache directory {:?}: {}", - self.cache.local_path, e + self.cache.directory, e ))); } @@ -227,10 +227,10 @@ impl ClientConfig { } } - /// Apply cache path override if provided - pub fn with_cache_path(mut self, cache_path: Option) -> Self { - if let Some(path) = cache_path { - self.cache.local_path = path; + /// Apply cache directory override if provided + pub fn with_cache_directory(mut self, cache_directory: Option) -> Self { + if let Some(path) = cache_directory { + self.cache.directory = path; } self } @@ -324,7 +324,7 @@ mod tests { assert!(args.endpoint.is_none()); assert!(args.timeout.is_none()); - assert!(args.cache_path.is_none()); + assert!(args.cache_directory.is_none()); assert!(!args.quiet); assert!(!args.no_shared_storage); assert!(args.transfer_chunk_size.is_none()); @@ -381,7 +381,7 @@ mod tests { config: None, endpoint: Some("http://cli-override:7777".to_string()), timeout: Some(120), - cache_path: None, + cache_directory: None, log_level: None, log_format: None, quiet: true, diff --git a/modelexpress_server/src/services.rs b/modelexpress_server/src/services.rs index cd6def1a..80b705a3 100644 --- a/modelexpress_server/src/services.rs +++ b/modelexpress_server/src/services.rs @@ -32,7 +32,7 @@ static START_TIME: std::sync::OnceLock = std::sync::OnceLock::new(); fn get_server_cache_dir() -> Option { // Try to get cache configuration if let Ok(config) = CacheConfig::discover() { - Some(config.local_path) + Some(config.directory) } else { // Fall back to environment variable std::env::var("HF_HUB_CACHE") diff --git a/test_client.sh b/test_client.sh index 927a1397..5384ea00 100644 --- a/test_client.sh +++ b/test_client.sh @@ -71,7 +71,7 @@ test_cache_cli() { # Test cache initialization print_status "Testing cache initialization..." - cargo run --bin cache_cli init --cache-path "$TEST_CACHE_PATH" || { + cargo run --bin cache_cli init --cache-directory "$TEST_CACHE_PATH" || { print_error "Cache initialization failed" return 1 } @@ -167,7 +167,7 @@ test_error_handling() { # Test with non-existent cache path print_status "Testing with non-existent cache path..." - cargo run --bin cache_cli --cache-path "/non/existent/path" list || { + cargo run --bin cache_cli --cache-directory "/non/existent/path" list || { print_success "Correctly handled non-existent cache path" } @@ -184,7 +184,7 @@ test_cache_discovery() { # Test command line argument discovery print_status "Testing command line argument discovery..." - cargo run --bin cache_cli --cache-path "$TEST_CACHE_PATH" list || { + cargo run --bin cache_cli --cache-directory "$TEST_CACHE_PATH" list || { print_error "Command line argument discovery failed" return 1 } @@ -202,7 +202,7 @@ test_cache_discovery() { print_status "Testing config file discovery..." mkdir -p ~/.model-express cat > ~/.model-express/config.yaml << EOF -local_path: $TEST_CACHE_PATH +directory: $TEST_CACHE_PATH server_endpoint: $SERVER_ENDPOINT timeout_secs: 30 EOF diff --git a/workspace-tests/tests/integration_tests.rs b/workspace-tests/tests/integration_tests.rs index 468de2ee..ad50eb2d 100644 --- a/workspace-tests/tests/integration_tests.rs +++ b/workspace-tests/tests/integration_tests.rs @@ -83,6 +83,7 @@ async fn test_integration_direct_download_invalid_model() { "definitely-not-a-real-model-name-12345", ModelProvider::HuggingFace, false, + None, ) .await; @@ -102,6 +103,7 @@ async fn test_integration_small_model_download() { "prajjwal1/bert-tiny", // A very small BERT model for testing ModelProvider::HuggingFace, false, + None, ) .await; @@ -153,6 +155,7 @@ async fn test_integration_offline_mode_without_cache() { "nonexistent-model-for-offline-test", ModelProvider::HuggingFace, false, + None, ) .await;