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

fix(rust, python): treat FSCK files_removed as strings #3219

Merged

Conversation

liamphmurphy
Copy link
Contributor

@liamphmurphy liamphmurphy commented Feb 14, 2025

Description

This changes how the files_removed array is serialized, making it a stringified representation of the list of files. This is done to ensure compatibility with Spark.

To maintain the current behavior with the python bindings, the repair method wrapper in python will deserialize back to a list.

Related Issue(s)

closes #3140

@github-actions github-actions bot added binding/python Issues for the Python package binding/rust Issues for the Rust crate labels Feb 14, 2025
Comment on lines 1435 to 1437
deserialized = json.loads(metrics)
deserialized[FSCK_METRICS_FILES_REMOVED_LABEL] = json.loads(deserialized["files_removed"])
return deserialized
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we don't care about maintaining the current response then we can omit this step, but for sake of breaking the interface, added a deserializing step here.

@liamphmurphy liamphmurphy force-pushed the fix/fsck_files_removed_as_strings branch 2 times, most recently from c2be848 to ff8cf2d Compare February 15, 2025 15:51
@ion-elgreco
Copy link
Collaborator

@liamphmurphy can you squash the commits please

@liamphmurphy liamphmurphy force-pushed the fix/fsck_files_removed_as_strings branch 2 times, most recently from 5b11979 to 4468c85 Compare February 15, 2025 16:32
@liamphmurphy
Copy link
Contributor Author

@liamphmurphy can you squash the commits please

Done! Let me know if something isn't looking right.

Copy link

codecov bot commented Feb 16, 2025

Codecov Report

Attention: Patch coverage is 42.85714% with 8 lines in your changes missing coverage. Please review.

Project coverage is 72.30%. Comparing base (2f1a5ac) to head (3f11d97).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
crates/core/src/operations/filesystem_check.rs 42.85% 7 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3219      +/-   ##
==========================================
- Coverage   72.31%   72.30%   -0.02%     
==========================================
  Files         138      138              
  Lines       45398    45412      +14     
  Branches    45398    45412      +14     
==========================================
+ Hits        32831    32833       +2     
- Misses      10489    10502      +13     
+ Partials     2078     2077       -1     

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

rtyler
rtyler previously approved these changes Feb 18, 2025
@rtyler rtyler force-pushed the fix/fsck_files_removed_as_strings branch from 6cbba07 to 3f11d97 Compare February 18, 2025 14:00
@rtyler rtyler enabled auto-merge February 18, 2025 14:01
@rtyler rtyler added this pull request to the merge queue Feb 18, 2025
auto-merge was automatically disabled February 18, 2025 14:33

Pull Request is not mergeable

Merged via the queue into delta-io:main with commit 76480f9 Feb 18, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/python Issues for the Python package binding/rust Issues for the Rust crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spark history read cannot deserialize metrics after FSCK Repair Run
3 participants