Skip to content
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

Create InvalidCheckpoint Error to bucket errors relevant to checkpoint #593

Merged
merged 5 commits into from
Dec 17, 2024

Conversation

Sevenannn
Copy link
Contributor

@Sevenannn Sevenannn commented Dec 12, 2024

What changes are proposed in this pull request?

Create a new variation of error: InvalidCheckpoint error to bucket errors relevant to invalid checkpoint files. This will allow for a more flexible handling of checkpoint related errors when using delta kernel crate.

How was this change tested?

Via cargo t --all-features --all-targets

@github-actions github-actions bot added the breaking-change Change that will require a version bump label Dec 12, 2024
@Sevenannn Sevenannn changed the title [WIP] Create InvalidCheckpoint Error to bucket errors relevant to checkpoint Create InvalidCheckpoint Error to bucket errors relevant to checkpoint Dec 12, 2024
@Sevenannn Sevenannn marked this pull request as ready for review December 12, 2024 02:44
Copy link
Collaborator

@scovich scovich left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for proposing this change, it looks great.

Copy link

codecov bot commented Dec 13, 2024

Codecov Report

Attention: Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.

Project coverage is 83.46%. Comparing base (ae841ea) to head (0b70459).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
kernel/src/error.rs 0.00% 3 Missing ⚠️
kernel/src/log_segment.rs 33.33% 2 Missing ⚠️
ffi/src/error.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #593      +/-   ##
==========================================
- Coverage   83.47%   83.46%   -0.01%     
==========================================
  Files          74       74              
  Lines       16889    16893       +4     
  Branches    16889    16893       +4     
==========================================
+ Hits        14098    14100       +2     
- Misses       2132     2135       +3     
+ Partials      659      658       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@zachschuermann zachschuermann left a comment

Choose a reason for hiding this comment

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

thanks this is a good idea - just a small nit but otherwise LGTM!

@roeap roeap mentioned this pull request Dec 14, 2024
@nicklan
Copy link
Collaborator

nicklan commented Dec 17, 2024

Tested msrv locally, not sure why the test is failing

@nicklan nicklan merged commit b6558d0 into delta-io:main Dec 17, 2024
18 of 21 checks passed
@nicklan
Copy link
Collaborator

nicklan commented Dec 17, 2024

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Change that will require a version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants