aokolnychyi commented on code in PR #6682:
URL: https://github.com/apache/iceberg/pull/6682#discussion_r1095234465
##########
api/src/main/java/org/apache/iceberg/actions/DeleteOrphanFiles.java:
##########
@@ -80,9 +84,16 @@ public interface DeleteOrphanFiles extends
Action<DeleteOrphanFiles, DeleteOrpha
*
* @param executorService the service to use
* @return this for method chaining
+ * @deprecated All deletes should be performed using the bulk delete api if
available. Use FileIO
+ * specific parallelism controls to adjust bulk delete concurrency
within that api.
*/
+ @Deprecated
DeleteOrphanFiles executeDeleteWith(ExecutorService executorService);
+ default DeleteOrphanFiles deleteBulkWith(Consumer<Iterable<String>>
deleteFunc) {
Review Comment:
My worry is that the current implementation breaks the current behavior in
an incompatible way: it checks if the table IO supports bulk deletes and if
yes, ignores `deleteWith`. It will break those dry-run scenarios if the table
is configured with a non-bulk IO and the user passes a custom delete function.
Can you elaborate a bit on why this is needed? Why not just not go through
the bulk delete API if a custom delete function is provided?
##########
api/src/main/java/org/apache/iceberg/actions/DeleteOrphanFiles.java:
##########
@@ -80,9 +84,16 @@ public interface DeleteOrphanFiles extends
Action<DeleteOrphanFiles, DeleteOrpha
*
* @param executorService the service to use
* @return this for method chaining
+ * @deprecated All deletes should be performed using the bulk delete api if
available. Use FileIO
+ * specific parallelism controls to adjust bulk delete concurrency
within that api.
*/
+ @Deprecated
DeleteOrphanFiles executeDeleteWith(ExecutorService executorService);
+ default DeleteOrphanFiles deleteBulkWith(Consumer<Iterable<String>>
deleteFunc) {
Review Comment:
My worry is that the current implementation breaks the behavior in an
incompatible way: it checks if the table IO supports bulk deletes and if yes,
ignores `deleteWith`. It will break those dry-run scenarios if the table is
configured with a non-bulk IO and the user passes a custom delete function.
Can you elaborate a bit on why this is needed? Why not just not go through
the bulk delete API if a custom delete function is provided?
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
