Skip to content

feat(iceberg): introduce remove snapshot action #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Mar 14, 2025

Conversation

Li0k
Copy link

@Li0k Li0k commented Mar 11, 2025

This PR adds the remove snapshot feature to the SDK along with basic tests. Note that currently, RW only relies on the main branch, no other branches or tags are involved.

refer to https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/RemoveSnapshots.java

Note: cleanExpiredMetadata is not currently implemented.


/// Finished building the action and apply it to the transaction.
pub async fn apply(self) -> Result<Transaction<'a>> {
self.apply_impl().await
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we can just put code in apply_impl in this function.🤔

}

/// Finished building the action and apply it to the transaction.
pub fn expire_snapshot_id(mut self, expire_snapshot_id: i64) -> Self {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we don't have test for this.

@Li0k Li0k requested review from ZENOTME and chenzl25 March 12, 2025 13:33
@Li0k Li0k marked this pull request as ready for review March 12, 2025 13:34
Copy link
Collaborator

@chenzl25 chenzl25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! So successful.

Copy link

@ZENOTME ZENOTME left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! Feel free to port this to upstream

@Li0k Li0k merged commit 2c50567 into dev_rebase_main_20250307 Mar 14, 2025
22 checks passed
@Li0k Li0k deleted the li0k/remove_snapshot_0307 branch March 14, 2025 06:10
Li0k added a commit that referenced this pull request Mar 25, 2025
Li0k added a commit that referenced this pull request Mar 25, 2025
* feat(iceberg): basic remove snapshot

* feat(iceberg): introduce new properties for remove snapshots

* feat(iceberg): support remove schemas

* refactor(iceberg): refactor file org

* address comments

* refactor(iceberg): refactor and ut

* fix(iceberg): fix integration-test
xxchan pushed a commit that referenced this pull request Mar 25, 2025
* feat(iceberg): basic remove snapshot

* feat(iceberg): introduce new properties for remove snapshots

* feat(iceberg): support remove schemas

* refactor(iceberg): refactor file org

* address comments

* refactor(iceberg): refactor and ut

* fix(iceberg): fix integration-test
xxchan pushed a commit that referenced this pull request Mar 25, 2025
* feat(iceberg): basic remove snapshot

* feat(iceberg): introduce new properties for remove snapshots

* feat(iceberg): support remove schemas

* refactor(iceberg): refactor file org

* address comments

* refactor(iceberg): refactor and ut

* fix(iceberg): fix integration-test
xxchan pushed a commit that referenced this pull request Mar 25, 2025
* feat(iceberg): basic remove snapshot

* feat(iceberg): introduce new properties for remove snapshots

* feat(iceberg): support remove schemas

* refactor(iceberg): refactor file org

* address comments

* refactor(iceberg): refactor and ut

* fix(iceberg): fix integration-test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants