Skip to content

Conversation

@aleksandarskrbic
Copy link
Contributor

@aleksandarskrbic aleksandarskrbic commented Sep 18, 2025

Fixes #1307

@aleksandarskrbic aleksandarskrbic changed the title Fixes #1307 remove storing UUID in LogPathFileType::UuidCheckpoint Sep 18, 2025
@github-actions github-actions bot added the breaking-change Change that require a major version bump label Sep 18, 2025
@zachschuermann zachschuermann changed the title remove storing UUID in LogPathFileType::UuidCheckpoint refactor: remove storing UUID in LogPathFileType::UuidCheckpoint Sep 18, 2025
Copy link
Collaborator

@OussamaSaoudi OussamaSaoudi left a comment

Choose a reason for hiding this comment

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

Just one nit :)

["checkpoint", uuid, "json" | "parquet"] => {
let uuid = parse_path_part(uuid, UUID_PART_LEN, url)?;
LogPathFileType::UuidCheckpoint(uuid)
let _ = parse_path_part::<String>(uuid, UUID_PART_LEN, url)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we actually care that it's a valid UUID if we never use it for anything but a filename?
(but the Delta spec does say it must be a UUID, so I guess we have to)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I think we might as well verify it

@codecov
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.79%. Comparing base (d3c7323) to head (01da21e).

Files with missing lines Patch % Lines
kernel/src/path.rs 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1317      +/-   ##
==========================================
- Coverage   84.81%   84.79%   -0.03%     
==========================================
  Files         113      113              
  Lines       28647    28642       -5     
  Branches    28647    28642       -5     
==========================================
- Hits        24297    24286      -11     
- Misses       3196     3197       +1     
- Partials     1154     1159       +5     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@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.

LGTM! thanks!

@zachschuermann zachschuermann merged commit 3637ebf into delta-io:main Oct 1, 2025
19 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Change that require a major version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

remove storing UUID in LogPathFileType::UuidCheckpoint

4 participants