RussellSpitzer commented on code in PR #6682:
URL: https://github.com/apache/iceberg/pull/6682#discussion_r1095922475
##########
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 goal is to remove the non-bulk delete api completely in the future. I
really don't want to be in a situation where we have some apis that only work
in conjunction with others.
I think if we allow `executeDeleteWith` and `deleteWith` we essentially have
this permanently split api. Where sometimes these functions are able to do
something but most of the time they can not. In the future I want to be in a
situation where there is only `deleteBulkWith`.
If we don't add `deleteBulkWith` but keep an alternate pathway with
`executeDeleteWith` and `deleteBulkWith` we will permanently need two sets of
tests. One testing the bulk pathway and one testing the non-bulk pathway. Which
pathway is used will also not be super clear as some procedures (if we keep dry
run as is) will invoke the non bulk pathway.
--
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]
