zhjwpku commented on code in PR #794:
URL: https://github.com/apache/iceberg-cpp/pull/794#discussion_r3503856049
##########
src/iceberg/util/error_collector.h:
##########
@@ -159,14 +159,20 @@ class ICEBERG_EXPORT ErrorCollector {
/// in Build(), Apply(), or Commit() methods) to validate that no errors
/// were accumulated during the builder method calls.
///
- /// \return Status indicating success if no errors, or a ValidationFailed
- /// error with all accumulated error messages
+ /// \return Status indicating success if no errors, a
RetryableValidationFailed if
+ /// all accumulated errors are retryable validations, or a
ValidationFailed
+ /// error with all accumulated error messages otherwise
[[nodiscard]] Status CheckErrors() const {
if (!errors_.empty()) {
std::string error_msg = "Validation failed due to the following
errors:\n";
+ bool all_retryable = true;
Review Comment:
I'm wondering why we require **all** the errors to be retryable before
treating the result as `RetryableValidationFailed`. Java impl throws a
`RetryableValidationException` immediately when it encounters a retryable
validation error, will this affect the behavior of lib users?
--
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]
