rdblue commented on a change in pull request #4071:
URL: https://github.com/apache/iceberg/pull/4071#discussion_r825199595
##########
File path: api/src/main/java/org/apache/iceberg/ManageSnapshots.java
##########
@@ -80,4 +80,107 @@
* wapId
*/
ManageSnapshots cherrypick(long snapshotId);
+
+ /**
+ * Create a new branch pointing to the given snapshot id.
+ *
+ * @param name branch name
+ * @param snapshotId id of the snapshot which will be the head of the branch
+ * @return this for method chaining
+ * @throws IllegalArgumentException if a branch with the given name already
exists
+ */
+ ManageSnapshots createBranch(String name, long snapshotId);
+
+ /**
+ * Create a new branch pointing to the given snapshot id with the given
retention properties.
+ *
+ * @param name branch name
+ * @param snapshotId id of the snapshot which will be the head of the branch
+ * @param maxRefAgeMs retention age in milliseconds of the branch reference
itself
+ * @param minSnapshotsToKeep minimum number of snapshots to retain on the
branch
+ * @param maxSnapshotAgeMs max age in milliseconds of a snapshot that should
be retained on the branch
+ * @return this for method chaining
+ * @throws IllegalArgumentException if a branch with the given name already
exists
+ */
+ ManageSnapshots createBranch(
+ String name,
+ long snapshotId,
+ Long maxRefAgeMs,
+ Integer minSnapshotsToKeep,
+ Long maxSnapshotAgeMs);
Review comment:
I think APIs that force you to pass in `null` are generally hard to use
and force you into code that is difficult to read. For example:
```java
table.manageSnapshots()
.createBranch("test", snapshotId, null, 100, null)
.commit();
```
I think it is better to use separate methods for each ref config setting:
```java
table.manageSnapshots()
.createBranch("test", snapshotId)
.setMinSnapshotsToKeep("test")
.commit();
```
I'd replace all of the variations of createTag/createBranch with simple ones
that only set the snapshot ID.
And, our general rule is that chained methods should produce the same result
in one commit that they would across multiple commits. So the above should be
equivalent to:
```java
table.manageSnapshots()
.createBranch("test", snapshotId)
.commit();
table.manageSnapshots()
.setMinSnapshotsToKeep("test")
.commit();
```
--
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]
