iceberg-python Feature: Write to branches

Fixes: https://github.com/apache/iceberg-python/issues/306

Jul 18 '24 17:07 vinjai

@sungwy @kevinjqliu This PR is ready for review

Oct 16 '24 08:10 vinjai

Fixed another bug. Writes with same name but different ref types were being successful. This is a bug in the current release version too

Please review whenever you get some time.

Oct 16 '24 20:10 vinjai

Thanks for the contribution! I'll take a look. I remember adding support for branch is complicated since we need to consider different edge cases.

Oct 19 '24 18:10 kevinjqliu

I have mostly tried to cover all edge cases. The idea is that the branch is just another iceberg table where the snapshots append independently of the main branch.

I also agree with your concern. If it helps, we can add more test cases

Oct 19 '24 19:10 vinjai

Hey @kevinjqliu Did you get a chance to look at this?

Nov 07 '24 22:11 vinjai

Thank you for the review @kevinjqliu

Nov 14 '24 01:11 vinjai

@kevinjqliu What are the next steps to get this merged?

Nov 20 '24 07:11 vinjai

cc @HonahX / @Fokko / @sungwy

Nov 21 '24 17:11 kevinjqliu

@vinjai coming back to this after the 0.8.1 release :) Feel free to tag me again once the comments above are addressed. Thanks again for the contribution!

Dec 02 '24 18:12 kevinjqliu

hey @vinjai are you interested to pick this back up?

Feb 01 '25 00:02 kevinjqliu

Hey @kevinjqliu I was traveling for the past two months and couldn’t complete the review comments. I’ll be back in a week, will address the comments then, and get this moving.

Feb 01 '25 13:02 vinjai

thank you! @vinjai feel free to tag me again for review :)

Feb 01 '25 16:02 kevinjqliu

Hey @kevinjqliu I have resolved all comments. Will resolve the merge conflicts once you have gone through these changes (to avoid rework).

Let me know once you have reviewed the above comments

Feb 23 '25 21:02 vinjai

Hey @kevinjqliu Did you get a chance to look at the PR again?

Mar 08 '25 06:03 vinjai

Hi, just wondering if there is an update on this PR?

Mar 22 '25 07:03 malcolmbovey

Hey @kevinjqliu Bumping up for review

Mar 26 '25 15:03 vinjai

Thanks @dbuades for pinging me, and sorry for letting this one linger for so long. Let me review this tomorrow morning.

@vinjai Do you have time to resolve the conflicts by any chance?

Apr 24 '25 20:04 Fokko

Hey @Fokko Will try to resolve the conflicts over the weekend.

Apr 28 '25 20:04 vinjai

Hey @vinjai, did you get a chance to fix those conflicts? 🙂

May 12 '25 08:05 SebastienN15

@vinjai I fixed the conflicts on this branch, feel free to cherry pick the last commit https://github.com/apache/iceberg-python/pull/2009 Otherwise I can open a PR on my side and take it from there 🙂

May 16 '25 14:05 SebastienN15

Thanks @SebastienN15 — I’ll review your commit and take it forward from there. I’ll share the working PR tomorrow.

May 23 '25 15:05 vinjai

Identified and fixed a bug related to empty tables. Planning to add test cases to cover this scenario.

May 26 '25 04:05 vinjai

Hey @Fokko This PR is ready for review again

May 27 '25 14:05 vinjai

Hey @Fokko I noticed that the integration tests for Python 3.9 are failing. Possibly, a runtime issue:

ServerError: NoSuchBucketException: The specified bucket does not exist (Service: S3, Status Code: 404, Request ID: 18436A88DE8BCC82, Extended Request ID: dd9025bab4ad464b049177c95eb6ebf374d3b3fd1af9251148b658df7ac2e3e8) (SDK Attempt Count: 1)

I ran the same tests locally with Python 3.9.6, and they passed without issues. Is there any change required in the PR to resolve the above failure?

Jun 01 '25 16:06 vinjai

@vinjai Thanks for pinging me. The issue has been fixed in https://github.com/apache/iceberg-python/pull/2049. Locally, you're still using an older version of minio, and therefore the tests pass. Could you pull in the latest main branch to fix the CI? Thanks!

Jun 01 '25 19:06 Fokko

Hey @Fokko I've pulled the latest changes from the main branch. Please re-trigger the workflow.

Jun 03 '25 09:06 vinjai

@vinjai I'm sorry, can you resolve the conflicts once more? I'll merge right after

Jun 18 '25 15:06 Fokko

Hey @vinjai, would you be able to resolve the conflicts? I would love to see this PR merged 🙏

Jun 23 '25 14:06 SebastienN15

Hey @Fokko I have resolved all conflicts and merged the recent changes. While running integration tests, I am facing this error for a few tests:

java.lang.IllegalStateException: Memory was leaked by query. Memory leaked: (360448)
Allocator(toArrowBatchIterator) 0/360448/360448/9223372036854775807 (res/actual/peak/limit)

The same error is coming on the main branch too.

Any idea on how to resolve this?

Jun 24 '25 01:06 vinjai

Hey @Fokko,

I tried the tests on a different machine. The tests are running fine. Request you to run the workflow and merge this PR ahead.

Thanks

Jun 28 '25 20:06 vinjai