crepererum commented on code in PR #376: URL: https://github.com/apache/arrow-rs-object-store/pull/376#discussion_r2108835862
########## src/azure/client.rs: ########## @@ -1540,4 +1565,135 @@ Time:2018-06-14T16:46:54.6040685Z</Message></Error>\r assert_eq!("404", code); assert_eq!("The specified blob does not exist.", reason); } + + #[tokio::test] + async fn test_list_blobs() { + let fake_properties = BlobProperties { + last_modified: Utc::now(), + content_length: 8, + content_type: "text/plain".to_string(), + content_encoding: None, + content_language: None, + e_tag: Some("etag".to_string()), + resource_type: Some("resource".to_string()), + }; + let fake_result = ListResultInternal { + prefix: None, + max_results: None, + delimiter: None, + next_marker: None, + blobs: Blobs { + blob_prefix: vec![], + blobs: vec![ + Blob { + name: "blob0.txt".to_string(), + version_id: None, + is_current_version: None, + deleted: None, + properties: fake_properties.clone(), + metadata: None, + }, + Blob { + name: "blob1.txt".to_string(), + version_id: None, + is_current_version: None, + deleted: None, + properties: fake_properties.clone(), + metadata: None, + }, + ], + }, + }; + let result = to_list_result(fake_result, None, false).unwrap(); + assert_eq!(result.common_prefixes.len(), 0); + assert_eq!(result.objects.len(), 2); + assert_eq!(result.objects[0].location, Path::from("blob0.txt")); + assert_eq!(result.objects[1].location, Path::from("blob1.txt")); + } + + #[tokio::test] + #[should_panic] Review Comment: ```suggestion #[should_panic(expected = "???")] ``` I find it helpful to check the panic message (or a substring of it) to ensure it's providing some useful user error. ########## src/azure/client.rs: ########## @@ -1058,6 +1063,9 @@ fn to_list_result(value: ListResultInternal, prefix: Option<&str>) -> Result<Lis !matches!(blob.properties.resource_type.as_ref(), Some(typ) if typ == "directory") && blob.name.len() > prefix.len() }) + .map(BlobInternal::try_from) + .filter(|parsed| !ignore_unparsable_paths || parsed.is_ok()) + .map(|parsed| parsed.unwrap()) Review Comment: I think instead of rolling this into two iterator steps, this would be easier to reason about: ```suggestion .filter_map(|parsed| { match parsed { Ok(parsed) => Some(parsed), Err(_) if ignore_unparsable_paths => None, Err(e) => panic!("cannot parse path: {e}"), } }) ``` -- 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...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org