danielcweeks commented on a change in pull request #22: ExpireSnapshots: do not
commit when no snapshots are expired.
URL: https://github.com/apache/incubator-iceberg/pull/22#discussion_r239538536
##########
File path: core/src/main/java/com/netflix/iceberg/RemoveSnapshots.java
##########
@@ -115,7 +115,10 @@ public void commit() {
.onlyRetryOn(CommitFailedException.class)
.run(item -> {
TableMetadata updated = internalApply();
- ops.commit(base, updated);
+ // only commit the updated metadata if at least one snapshot was
removed
+ if (updated.snapshots().size() != base.snapshots().size()) {
Review comment:
I think those two checks are functionally equivalent for this check. This
is to effectively check that the number of snapshots has not changed. I would
err on the side of equality as the proposed change would conceptually mean that
you could increase the number of snapshots, which would not be expected in this
case.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
