amogh-jahagirdar commented on code in PR #14287:
URL: https://github.com/apache/iceberg/pull/14287#discussion_r2547088874
##########
core/src/main/java/org/apache/iceberg/RemoveSnapshots.java:
##########
@@ -350,9 +367,11 @@ public void commit() {
TableMetadata updated = internalApply();
ops.commit(base, updated);
});
- LOG.info("Committed snapshot changes");
+ LOG.info(
+ "Committed snapshot changes and prepare to clean up files at level={}",
+ cleanupLevel.name());
- if (cleanExpiredFiles && !base.snapshots().isEmpty()) {
Review Comment:
Minor: I would've preferred for this to just change to cleanupLevel !=
CleanupLevel.NONE instead of nesting the logic further inside
cleanExpiredSnapshots. Now someone has to read cleanExpiredSnapshots() to see
it gets skipped when it's none, whereas I feel like it's better if
cleanExpiredSnapshots just does cleanup, and we skip before that point.
--
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]
