stevenzwu commented on code in PR #14287:
URL: https://github.com/apache/iceberg/pull/14287#discussion_r2449831339
##########
core/src/main/java/org/apache/iceberg/RemoveSnapshots.java:
##########
@@ -103,7 +104,12 @@ class RemoveSnapshots implements ExpireSnapshots {
@Override
public ExpireSnapshots cleanExpiredFiles(boolean clean) {
- this.cleanExpiredFiles = clean;
+ LOG.warn("cleanExpiredFiles(boolean) is deprecated. Use
cleanMode(CleanupMode) instead.");
+ Preconditions.checkArgument(
Review Comment:
but the condition may not be precise/perfect. e.g., if the client called
`cleanupLevel(ALL)` (default value), it would still allow this method to go
through.
the other way is to default `cleanupLevel` to `null`. but we would need to
do a bit if-else check during read to apply the value.
maybe the complexity is not worth it. I am wondering if it is simpler to
just go with what @nastra suggested. just rely on API deprecation to move users
away from the old API.
--
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]
