fix for #573 corrected error message in case of deployment mismatch

    data directory creation should not happen in case of deployment mismatch
    staging should be overwritten in case of new staging
    default staging and data directory should not be created if env var has different path

Author: nikhilsinhaparseable

Cargo.lock

@@ -102,6 +102,7 @@ url = "2.4.0"
 http-auth-basic = "0.3.3"
 serde_repr = "0.1.17"
 hashlru = { version = "0.11.0", features = ["serde"] }
+path-clean = "1.0.1"
 
 [build-dependencies]
 cargo_toml = "0.15"

server/Cargo.toml

@@ -55,10 +55,9 @@ use crate::localcache::LocalCacheManager;
 async fn main() -> anyhow::Result<()> {
     env_logger::init();
     let storage = CONFIG.storage().get_object_store();
-    CONFIG.validate_staging()?;
     migration::run_metadata_migration(&CONFIG).await?;
     let metadata = storage::resolve_parseable_metadata().await?;
-
+    CONFIG.validate_staging()?;
     banner::print(&CONFIG, &metadata).await;
     rbac::map::init(&metadata);
     metadata.set_global();

server/src/main.rs

@@ -21,7 +21,8 @@ use clap::{command, value_parser, Arg, ArgGroup, Args, Command, FromArgMatches};
 
 use once_cell::sync::Lazy;
 use parquet::basic::{BrotliLevel, GzipLevel, ZstdLevel};
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
+use std::env;
 use std::sync::Arc;
 use url::Url;
 
@@ -107,7 +108,7 @@ impl Config {
         self.storage.clone()
     }
 
-    pub fn staging_dir(&self) -> &Path {
+    pub fn staging_dir(&self) -> &PathBuf {
         &self.parseable.local_staging_path
     }
 
@@ -610,12 +611,14 @@ impl From<Compression> for parquet::basic::Compression {
 
 pub mod validation {
     use std::{
-        fs::{canonicalize, create_dir_all},
         net::ToSocketAddrs,
-        path::PathBuf,
+        path::{PathBuf, Path},
         str::FromStr,
+        env,io
     };
 
+    use path_clean::PathClean;
+
     use crate::option::MIN_CACHE_SIZE_BYTES;
     use crate::storage::LOCAL_SYNC_INTERVAL;
     use human_size::{multiples, SpecificSize};
@@ -633,16 +636,22 @@ pub mod validation {
 
         Ok(path)
     }
+    pub fn absolute_path(path: impl AsRef<Path>) -> io::Result<PathBuf> {
+        let path = path.as_ref();
+    
+        let absolute_path = if path.is_absolute() {
+            path.to_path_buf()
+        } else {
+            env::current_dir()?.join(path)
+        }.clean();
+    
+        Ok(absolute_path)
+    }
 
     pub fn canonicalize_path(s: &str) -> Result<PathBuf, String> {
         let path = PathBuf::from(s);
-
-        create_dir_all(&path)
-            .map_err(|err| err.to_string())
-            .and_then(|_| {
-                canonicalize(&path)
-                    .map_err(|_| "Cannot use the path provided as an absolute path".to_string())
-            })
+        let absolute_path = absolute_path(&path);
+        Ok(absolute_path.unwrap())
     }
 
     pub fn socket_addr(s: &str) -> Result<String, String> {

server/src/option.rs

@@ -32,7 +32,7 @@ use tokio::fs::{self, DirEntry};
 use tokio_stream::wrappers::ReadDirStream;
 
 use crate::metrics::storage::{localfs::REQUEST_RESPONSE_TIME, StorageMetrics};
-use crate::utils::validate_path_is_writeable;
+use crate::{option::validation, utils::validate_path_is_writeable};
 
 use super::{object_storage, LogStream, ObjectStorage, ObjectStorageError, ObjectStorageProvider};
 
@@ -49,7 +49,8 @@ pub struct FSConfig {
     #[arg(
         env = "P_FS_DIR",
         value_name = "filesystem path",
-        default_value = "./data"
+        default_value = "./data",
+        value_parser = validation::canonicalize_path
     )]
     pub root: PathBuf,
 }

server/src/storage/localfs.rs

@@ -66,7 +66,7 @@ impl StorageMetadata {
         Self {
             version: "v3".to_string(),
             mode: CONFIG.storage_name.to_owned(),
-            staging: CONFIG.staging_dir().canonicalize().unwrap(),
+            staging: CONFIG.staging_dir().to_path_buf(),
             storage: CONFIG.storage().get_endpoint(),
             deployment_id: uid::gen(),
             users: Vec::new(),
@@ -130,7 +130,7 @@ pub async fn resolve_parseable_metadata() -> Result<StorageMetadata, ObjectStora
         }
         EnvChange::CreateBoth => {
             create_dir_all(CONFIG.staging_dir())?;
-            let metadata = StorageMetadata::new();
+           let metadata = StorageMetadata::new();
             // new metadata needs to be set on both staging and remote
             overwrite_remote = true;
             overwrite_staging = true;