Xuanwo commented on code in PR #1383:
URL: https://github.com/apache/iceberg-rust/pull/1383#discussion_r2111790766
##########
crates/iceberg/src/error.rs:
##########
@@ -134,6 +134,8 @@ pub struct Error {
source: Option<anyhow::Error>,
backtrace: Backtrace,
+
+ retryable: bool,
Review Comment:
> Whether it's retryable should be determined by consumer by `ErrorKind`.
My current idea is to introduce an `ErrorStatus`:
- `temporary`: This error is temporary, and users can retry the operation if
they want.
- `permanent`: This error is permanent, so users can't retry it. Even if
they do, it's highly likely that it won't work.
In this way, we convey the status, which is only known to us and is
orthogonal of `ErrorKind` instead of the final decision, `retryable`.
For example, `Unexpected` error can be `permanent` and `PreconditionFailed`
error can be `temporary`.
This is also very useful in cases like our REST catalog client. We can offer
built-in retry support and convert a `temporary` error into a `permanent` one
after several retry attempts.
---
I also think it would be good to use a `temporary: bool` instead.
--
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]
