[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #968: Fix SnapshotManager so that snapshotId() returns correct value

[GitBox <gi...@apache.org>]

rdblue commented on a change in pull request #968:
URL: https://github.com/apache/incubator-iceberg/pull/968#discussion_r415404146



##########
File path: core/src/main/java/org/apache/iceberg/SnapshotManager.java
##########
@@ -50,25 +51,34 @@ protected ManageSnapshots self() {
 
   @Override
   protected String operation() {
-    // snapshotOperation is used by SnapshotProducer when building and writing a new snapshot for cherrypick
-    Preconditions.checkNotNull(snapshotOperation, "[BUG] Detected uninitialized operation");
+    //TODO: We are not setting this var during rollbacks. What should its value be during rollbacks?

Review comment:
       A rollback doesn't create a snapshot, it uses an existing one. In that case, `operation` is never called to create summary metadata. That's why there was the bug check here.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org