Review comments

earth-mover · Aug 15, 2024 · c1f1b16 · c1f1b16
1 parent ef51b3a
commit c1f1b16
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 8 deletions.
diff --git a/src/dataset.rs b/src/dataset.rs
@@ -621,7 +621,10 @@ mod tests {
             Arc::new(ObjectStorage::new_in_memory_store()),
             Arc::new(ObjectStorage::new_local_store(Path::new(&temp_dir_name)).unwrap()),
             // Arc::new(ObjectStorage::new_s3_store_from_env("foo".to_string()).unwrap()),
-            Arc::new(ObjectStorage::new_s3_store_with_config("foo".to_string()).unwrap()),
+            Arc::new(
+                ObjectStorage::new_s3_store_with_config("testbucket".to_string())
+                    .unwrap(),
+            ),
         ];
         for storage in storages {
             let array_id = 2;
@@ -913,7 +916,10 @@ mod tests {
             Arc::new(ObjectStorage::new_in_memory_store()),
             Arc::new(ObjectStorage::new_local_store(Path::new(&temp_dir_name)).unwrap()),
             // Arc::new(ObjectStorage::new_s3_store_from_env("foo".to_string()).unwrap()),
-            Arc::new(ObjectStorage::new_s3_store_with_config("testbucket".to_string()).unwrap()),
+            Arc::new(
+                ObjectStorage::new_s3_store_with_config("testbucket".to_string())
+                    .unwrap(),
+            ),
         ];
         for storage in storages {
             let mut ds = Dataset::create(Arc::clone(&storage));

diff --git a/src/storage.rs b/src/storage.rs
@@ -54,18 +54,18 @@ impl ObjectStorage {
         })
     }
     pub fn new_s3_store_from_env(
-        bucket_name: String,
+        bucket_name: impl Into<String>,
     ) -> Result<ObjectStorage, StorageError> {
         use object_store::aws::AmazonS3Builder;
         let store = AmazonS3Builder::from_env()
-            .with_bucket_name(bucket_name)
+            .with_bucket_name(bucket_name.into())
             .build()
             .map_err(|err| StorageError::UrlParseError(Box::new(err)))?;
         Ok(ObjectStorage { store: Arc::new(store) })
     }
 
     pub fn new_s3_store_with_config(
-        bucket_name: String,
+        bucket_name: impl Into<String>,
     ) -> Result<ObjectStorage, StorageError> {
         use object_store::aws::AmazonS3Builder;
         let store = AmazonS3Builder::new()
@@ -74,14 +74,13 @@ impl ObjectStorage {
             .with_secret_access_key("minio123")
             .with_endpoint("http://localhost:9000")
             .with_allow_http(true)
-            .with_bucket_name(bucket_name)
+            .with_bucket_name(bucket_name.into())
             .build()
             .map_err(|err| StorageError::UrlParseError(Box::new(err)))?;
         Ok(ObjectStorage { store: Arc::new(store) })
     }
 
-    fn get_path(filetype: FileType, id: &ObjectId) -> Path {
-        let ObjectId(asu8) = id;
+    fn get_path(filetype: FileType, ObjectId(asu8): &ObjectId) -> Path {
         let prefix = filetype.get_prefix();
         // TODO: be careful about allocation here
         let path = format!("{}/{}", prefix, BASE64_URL_SAFE.encode(asu8));