pvary commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2724153881
LGTM +1
Let's wait a bit, so if anyone would like to comment, they can.
Will merge next week, unless there are any objections in the meantime
--
This is an automated message from t
gaborkaszab commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1993175661
##
core/src/main/java/org/apache/iceberg/FileCleanupStrategy.java:
##
@@ -23,15 +23,25 @@
import java.util.function.Consumer;
import org.apache.iceberg.avro.Avr
pvary commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1993268022
##
core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java:
##
@@ -1621,6 +1627,82 @@ public void testRetainFilesOnRetainedBranches() {
assertThat(deletedFi
gaborkaszab commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1993039477
##
core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java:
##
@@ -1621,6 +1627,82 @@ public void testRetainFilesOnRetainedBranches() {
assertThat(del
pvary commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1989419495
##
core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java:
##
@@ -1621,6 +1627,82 @@ public void testRetainFilesOnRetainedBranches() {
assertThat(deletedFi
pvary commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2714512533
I'm fine with the current approach.
Could you please rebase, and address the little nit?
Thanks,
Peter
--
This is an automated message from the Apache Git Service.
To respond to
pvary commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1989420919
##
core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java:
##
@@ -1621,6 +1627,82 @@ public void testRetainFilesOnRetainedBranches() {
assertThat(deletedFi
pvary commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1989403001
##
core/src/main/java/org/apache/iceberg/FileCleanupStrategy.java:
##
@@ -23,15 +23,25 @@
import java.util.function.Consumer;
import org.apache.iceberg.avro.Avro;
im
gaborkaszab commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2693931184
> Why did you decide against an util method? The code is really very similar
to the orphan file removal stuff
I figured that first we should agree on the approach, because las
pvary commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2690965941
Why did you decide against an util method? The code is really very similar
to the orphan file removal stuff
--
This is an automated message from the Apache Git Service.
To respond to th
gaborkaszab commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2690864221
Hi @amogh-jahagirdar , @pvary ,
I managed to simplify the original PR and uploaded a new version. I took a
deeper look at how [orphan file deletion does the same
thing](https
gaborkaszab commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1955942576
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or mo
gaborkaszab commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1955843549
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or mo
gaborkaszab commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2658767328
Thanks for taking a look @pvary, @amogh-jahagirdar and @steveloughran ! I'll
be offline for a couple of days but will take a deeper look after that.
Another thing we discussed
gaborkaszab commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1955828374
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or mo
gaborkaszab commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1955843549
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or mo
gaborkaszab commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1955828374
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or mo
gaborkaszab commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1955819540
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or mo
amogh-jahagirdar commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1950075730
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ *
amogh-jahagirdar commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2649544429
Sorry for the late followup, I'm taking a look!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
UR
pvary commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1949864076
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more con
steveloughran commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1949606394
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or
pvary commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1949131366
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more con
pvary commented on code in PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#discussion_r1949121981
##
core/src/main/java/org/apache/iceberg/BulkDeleteConsumer.java:
##
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more con
gaborkaszab commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2647626948
@amogh-jahagirdar Would you mind taking a look. This came from a Slack
discussion we had earlier.
cc @pvary in case you have some capacity for this.
Thanks!
--
This is a
gaborkaszab commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2604978058
Hi @amogh-jahagirdar ,
Would you mind taking a look? This PR came up with a conversation with you
on Iceberg Slack.
https://apache-iceberg.slack.com/archives/C03LG1D563F/p1733215
gaborkaszab commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2604973773
> I'm going to suggest some tests of failure handling to see what happens
there
>
> * missing file (all should ignore)
> * putting a non-empty directory where one of the fi
steveloughran commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2592695395
I'm going to suggest some tests of failure handling to see what happens there
* missing file (all should ignore)
* putting a non-empty directory where one of the file path
gaborkaszab commented on PR #11837:
URL: https://github.com/apache/iceberg/pull/11837#issuecomment-2557371186
Slack discussion about this:
https://apache-iceberg.slack.com/archives/C03LG1D563F/p1733215233582339
--
This is an automated message from the Apache Git Service.
To respond to the
29 matches
Mail list logo