Xuanwo commented on code in PR #1383:
URL: https://github.com/apache/iceberg-rust/pull/1383#discussion_r2113826850


##########
crates/iceberg/src/error.rs:
##########
@@ -134,6 +134,8 @@ pub struct Error {
 
     source: Option<anyhow::Error>,
     backtrace: Backtrace,
+
+    retryable: bool,

Review Comment:
   > I don't quite understand why `PreconditionFailed` can be temporrary.
   
   In cases like concurrent commit. Users can retry until this operation 
succeed or give up while retry times reached.
   
   Take S3 as an example: 
   
   https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html
   
   > ConditionalRequestConflict
   >
   > A conflicting operation occurred. If using PutObject you can retry the 
request. If using multipart upload you should initiate another 
CreateMultipartUpload request and re-upload each part.
   
   ---
   
   > If we add the `status` or `temporary` field, it's the function telling its 
caller whether they should retry.
   
   I think that's exactly why I want to use "temporary" here. We're telling 
users that this error is temporary, allowing them to decide how to proceed 
based on their own context.
   
   A temporary error doesn't necessarily mean it should be retried.



-- 
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