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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
