nastra commented on code in PR #10995:
URL: https://github.com/apache/iceberg/pull/10995#discussion_r1815163457
##########
core/src/test/java/org/apache/iceberg/TestFastAppend.java:
##########
@@ -252,11 +252,36 @@ public void testFailure() {
assertThat(new File(newManifest.path())).doesNotExist();
}
+ @TestTemplate
+ public void testIncreaseNumRetries() {
+ TestTables.TestTableOperations ops = table.ops();
+ ops.failCommits(TableProperties.COMMIT_NUM_RETRIES_DEFAULT + 1);
+
+ AppendFiles append = table.newFastAppend().appendFile(FILE_B);
+
+ // Default number of retries results in a failed commit
+ assertThatThrownBy(append::commit)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessage("Injected failure");
+
+ // After increasing the number of retries the commit succeeds
+ table
+ .updateProperties()
+ .set(
+ TableProperties.COMMIT_NUM_RETRIES,
+ String.valueOf(TableProperties.COMMIT_NUM_RETRIES_DEFAULT + 1))
+ .commit();
+
+ append.commit();
+
+ validateSnapshot(null, readMetadata().currentSnapshot(), FILE_B);
+ }
+
@TestTemplate
public void testAppendManifestCleanup() throws IOException {
- // inject 5 failures
+ // inject failures
TestTables.TestTableOperations ops = table.ops();
- ops.failCommits(5);
+ ops.failCommits(TableProperties.COMMIT_NUM_RETRIES_DEFAULT + 1);
Review Comment:
my main issue with these test updates is that those are not related to the
scope of the PR itself, so I'd suggest to revert these changes. Adding a
separate test method (which you did with `testIncreaseNumRetries()`) is fine
but generally we try to avoid updating existing tests for no particular reason.
Also I believe the goal of these tests was to use a very specific number of
failures and such (independent from the default)
--
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]
