Code review feedback

Author: mpiannucci

icechunk/src/storage/object_store.rs

@@ -279,7 +279,7 @@ impl private::Sealed for ObjectStorage {}
 #[typetag::serde]
 impl Storage for ObjectStorage {
     fn can_write(&self) -> bool {
-        true
+        self.backend.can_write()
     }
 
     #[instrument(skip_all)]
@@ -654,6 +654,10 @@ pub trait ObjectStoreBackend: Debug + Display + Sync + Send {
     }
 
     fn default_settings(&self) -> Settings;
+
+    fn can_write(&self) -> bool {
+        true
+    }
 }
 
 #[derive(Debug, Serialize, Deserialize)]
@@ -785,7 +789,7 @@ impl fmt::Display for HttpObjectStoreBackend {
 impl ObjectStoreBackend for HttpObjectStoreBackend {
     fn mk_object_store(
         &self,
-        _settings: &Settings,
+        settings: &Settings,
     ) -> Result<Arc<dyn ObjectStore>, StorageError> {
         let builder = HttpBuilder::new().with_url(&self.url);
 
@@ -797,6 +801,20 @@ impl ObjectStoreBackend for HttpObjectStoreBackend {
             .iter()
             .fold(builder, |builder, (key, value)| builder.with_config(*key, value));
 
+        let builder = builder.with_retry(RetryConfig {
+            backoff: BackoffConfig {
+                init_backoff: core::time::Duration::from_millis(
+                    settings.retries().initial_backoff_ms() as u64,
+                ),
+                max_backoff: core::time::Duration::from_millis(
+                    settings.retries().max_backoff_ms() as u64,
+                ),
+                base: 2.,
+            },
+            max_retries: settings.retries().max_tries().get() as usize - 1,
+            retry_timeout: core::time::Duration::from_secs(5 * 60),
+        });
+
         let store =
             builder.build().map_err(|e| StorageErrorKind::Other(e.to_string()))?;
 
@@ -810,6 +828,11 @@ impl ObjectStoreBackend for HttpObjectStoreBackend {
     fn default_settings(&self) -> Settings {
         Default::default()
     }
+
+    fn can_write(&self) -> bool {
+        // TODO: Support write operations?
+        false
+    }
 }
 
 #[derive(Debug, Serialize, Deserialize)]

icechunk/src/virtual_chunks.rs

@@ -63,6 +63,7 @@ impl VirtualChunkContainer {
                 Err("in memory storage does not accept credentials".to_string())
             }
             (ObjectStoreConfig::Http(_), _) => {
+                // TODO: Support basic and bearer auth
                 Err("http storage does not support credentials".to_string())
             }
             (ObjectStoreConfig::S3Compatible(_), Credentials::S3(_)) => Ok(()),
@@ -137,14 +138,6 @@ pub fn mk_default_containers() -> HashMap<ContainerName, VirtualChunkContainer>
                 store: ObjectStoreConfig::Http(HashMap::new()),
             },
         ),
-        (
-            "https".to_string(),
-            VirtualChunkContainer {
-                name: "https".to_string(),
-                url_prefix: "https".to_string(),
-                store: ObjectStoreConfig::Http(HashMap::new()),
-            },
-        ),
     ]
     .into_iter()
     .collect()