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