Jefffrey commented on code in PR #17231:
URL: https://github.com/apache/datafusion/pull/17231#discussion_r2285213235


##########
datafusion/core/tests/tracing/mod.rs:
##########
@@ -76,7 +76,12 @@ async fn run_query() {
     let ctx = SessionContext::new();
 
     // Get the test data directory
-    let test_data = parquet_test_data();
+    let mut test_data = parquet_test_data();
+    if cfg!(target_os = "windows") {
+        // Replace backslashes (Windows paths) as they are invalid: 
https://datatracker.ietf.org/doc/html/rfc8089#appendix-E.4
+        // Also add "/", as Windows paths start with <Drive>:/ instead of "/"
+        test_data = format!("/{}", test_data.replace("\\", "/"))
+    }

Review Comment:
   Would it be better to fix this at the root cause, aka inside 
`parquet_test_data()` instead of patching it after the fact here?



##########
datafusion/core/tests/parquet/encryption.rs:
##########
@@ -284,7 +284,15 @@ fn verify_file_encrypted(
     options
         .options
         .insert("test_key".to_string(), "test value".to_string());
-    let object_path = 
object_store::path::Path::from(file_path.to_str().unwrap());
+
+    // the paths in encryption_factory are stored with "/", so we need to 
replace them on Windows
+    let file_path_str = if cfg!(target_os = "windows") {
+        file_path.to_str().unwrap().replace("\\", "/")
+    } else {
+        file_path.to_str().unwrap().to_owned()
+    };

Review Comment:
   This confuses me a bit, specifically the comment as I don't think 
`encryption_factory` has anything to do with it? Is it possible to look for 
root cause of whats passing a Unix path to this function instead of patching 
here? 🤔 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to