ZENOTME commented on code in PR #1383:
URL: https://github.com/apache/iceberg-rust/pull/1383#discussion_r2113823353
##########
crates/iceberg/src/error.rs:
##########
@@ -134,6 +134,8 @@ pub struct Error {
source: Option<anyhow::Error>,
backtrace: Backtrace,
+
+ retryable: bool,
Review Comment:
> If we add the status or temporary field, it's the function telling its
caller whether they should retry. My concern is that adding such a field a
general purpose Error could be misleading or confusing.
This example makes sense to me.🤔 There are two kinds of retry process: 1.
commit conflict, 2. other(maybe network timeout as @liurenjie1024 says). It
seems that in the SDK, at least for now, we only try to retry the process when
we meet CommitConflict. (In Java, exceptions only retry when they meet
CommitFailedException). For other errors, maybe it should be determined by the
user. And we can determine whether need to let retry orthogonal of ErrorKind
until we have multiple kinds of errors that need to 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]
