CTTY commented on code in PR #1400:
URL: https://github.com/apache/iceberg-rust/pull/1400#discussion_r2125576342


##########
crates/iceberg/src/transaction/mod.rs:
##########
@@ -185,19 +195,111 @@ impl<'a> Transaction<'a> {
 
     /// Set the location of table
     pub fn set_location(mut self, location: String) -> Result<Self> {
-        self.apply(vec![TableUpdate::SetLocation { location }], vec![])?;
+        let set_location = SetLocationAction::new().set_location(location);
+        Arc::new(set_location).commit(&mut self)?;
         Ok(self)
     }
 
     /// Commit transaction.
-    pub async fn commit(self, catalog: &dyn Catalog) -> Result<Table> {
+    pub async fn commit(&mut self, catalog: Arc<&dyn Catalog>) -> 
Result<Table> {
+        if self.actions.is_empty()
+            || (self.base_table.metadata() == self.current_table.metadata()
+                && self.base_table.metadata_location() == 
self.current_table.metadata_location())
+        {
+            // nothing to commit
+            return Ok(self.current_table.clone());
+        }
+
+        // let retry_strategy = ExponentialBackoff::from_millis(10)
+        //     .take(3);    // limit to 3 retries
+
+        // Retry::spawn(retry_strategy,
+        //             || {
+        //             async {
+        //                 Transaction::do_commit(
+        //                     &mut self.clone(),
+        //                     catalog.clone()).await
+        //             }
+        //         }).await
+

Review Comment:
   I tried both 
[tokio-retry2](https://docs.rs/tokio-retry2/latest/tokio_retry2/index.html) and 
[backon](https://docs.rs/backon/latest/backon/).
   
    Both will require us toss around an intermediate Error type (`RetryError` 
for tokio-retry2 and `anyhow::Error` for backon). `backon` integrates better 
with my existing code, which requires `&mut self` in the `do_commit` method.  
Also, somewhat surprisingly, `tokio-retry2` doesn't provide `min_delay`. 
   
   I've chosen to go with `backon` here but would appreciate any other feedback



-- 
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: issues-unsubscr...@iceberg.apache.org

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


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

Reply via email to