gaborkaszab commented on code in PR #10995:
URL: https://github.com/apache/iceberg/pull/10995#discussion_r1768454689
##########
core/src/main/java/org/apache/iceberg/FastAppend.java:
##########
@@ -24,20 +24,14 @@
import java.util.Set;
import org.apache.iceberg.encryption.EncryptedOutputFile;
import org.apache.iceberg.events.CreateSnapshotEvent;
-import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.exceptions.RuntimeIOException;
import org.apache.iceberg.io.InputFile;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.util.CharSequenceSet;
-/**
- * {@link AppendFiles Append} implementation that adds a new manifest file for
the write.
- *
- * <p>This implementation will attempt to commit 5 times before throwing {@link
Review Comment:
Thanks for taking a look, @nastra!
Frankly, I don' really prefer redundant comments. I don't see why we'd want
to add the same comment for all the subclasses of SnapshotProducer. First, as
said in my description retries don't happen on that level of abstraction. Let's
say for some reason the retry logic or the name of the config changes in the
future. Then we have to remember that there is a comment about this in all the
subclasses that we have to change now too.
What if I add a comment about the retries to SnapshotProducer itself? That's
the level of abstraction where that logic happens, would be more suitable and
we wouldn't have redundant comments in the subclasses.
--
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
