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.

Sign up for GitHub

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

Jump to bottom

refactor: delta, delta-polars #16

Merged
merged 1 commit into from
Jan 22, 2025
Merged

refactor: delta, delta-polars #16

merged 1 commit into from
Jan 22, 2025

Conversation

ion-elgreco
Copy link
Member

  • Resolved all deprecations in the code base
  • Sets writer engine to Rust as default to mimic deltalake behavior
  • Returns pa.Table instead of reader in to_arrow() for preparation of release 0.24.1 in deltalake
  • Deprecates lakefs IO Manager since there is native support now
  • Handles lakefs uri to lakefs link conversion

@ion-elgreco ion-elgreco force-pushed the refactor--deltalake-polars branch from 2c80339 to 4b202af Compare January 22, 2025 16:03
@ion-elgreco ion-elgreco enabled auto-merge (squash) January 22, 2025 16:05
@ion-elgreco ion-elgreco requested a review from gfvioli January 22, 2025 16:05
@ion-elgreco ion-elgreco force-pushed the refactor--deltalake-polars branch from 3b907cf to beb906e Compare January 22, 2025 19:24
gfvioli
gfvioli reviewed Jan 22, 2025
View reviewed changes
Copy link
Collaborator

@gfvioli gfvioli left a comment

Choose a reason for hiding this comment

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

Apart from the duplication of the s3 uri converter, I think is all good. Let me know your opinion on the comment and we can approve tomorrow.

gfvioli
gfvioli approved these changes Jan 22, 2025
View reviewed changes
Copy link
Collaborator

@gfvioli gfvioli left a comment

Choose a reason for hiding this comment

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

Approving as per agreement to leave the duplicated code upon the deprecation of the lakefs IO manager.

@ion-elgreco ion-elgreco disabled auto-merge January 22, 2025 19:30
@ion-elgreco ion-elgreco merged commit 55f6245 into main Jan 22, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gfvioli gfvioli gfvioli approved these changes

@sverbruggen sverbruggen Awaiting requested review from sverbruggen sverbruggen is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ion-elgreco @gfvioli