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

feat(disable conflict checker): start counting concurrent retries at 0 and disable conflict checker with maxRetries=0 #3204

Merged
merged 2 commits into from
Feb 14, 2025

Conversation

daphnenhuch-at
Copy link
Contributor

@daphnenhuch-at daphnenhuch-at commented Feb 10, 2025

Description

Add behavior such that max_retries = 0 does not enable the conflict checker

Related Issue(s)

Closes #3186 3186

Documentation

n/a

@github-actions github-actions bot added the binding/rust Issues for the Rust crate label Feb 10, 2025
Copy link

ACTION NEEDED

delta-rs follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

@daphnenhuch-at daphnenhuch-at changed the title Add option to disable conflict checker in CommitProperties feat:Add option to disable conflict checker in CommitProperties Feb 10, 2025
@daphnenhuch-at daphnenhuch-at changed the title feat:Add option to disable conflict checker in CommitProperties feat: Add option to disable conflict checker in CommitProperties Feb 11, 2025
@daphnenhuch-at
Copy link
Contributor Author

Question: When do you expect the next official release of deltalake to be released?

Copy link

codecov bot commented Feb 12, 2025

Codecov Report

Attention: Patch coverage is 93.47826% with 3 lines in your changes missing coverage. Please review.

Project coverage is 72.14%. Comparing base (b3efdfc) to head (cdd2610).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
crates/core/src/operations/write.rs 92.30% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3204      +/-   ##
==========================================
+ Coverage   72.10%   72.14%   +0.03%     
==========================================
  Files         138      138              
  Lines       45320    45365      +45     
  Branches    45320    45365      +45     
==========================================
+ Hits        32678    32728      +50     
+ Misses      10567    10565       -2     
+ Partials     2075     2072       -3     

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

@ion-elgreco
Copy link
Collaborator

Looks good, can you add a test for this new behaviour please?

@daphnenhuch-at
Copy link
Contributor Author

Added a test!

@daphnenhuch-at daphnenhuch-at changed the title feat: Add option to disable conflict checker in CommitProperties feat: Start counting concurrent retries at 0 and disable conflict checker with maxRetries=0 Feb 12, 2025
@daphnenhuch-at daphnenhuch-at changed the title feat: Start counting concurrent retries at 0 and disable conflict checker with maxRetries=0 feat(disable conflict checker): start counting concurrent retries at 0 and disable conflict checker with maxRetries=0 Feb 12, 2025
@ion-elgreco
Copy link
Collaborator

@daphnenhuch-at looks good, can you squash the commits into one please. Then we can merge it

@daphnenhuch-at daphnenhuch-at force-pushed the main branch 2 times, most recently from 31c60df to a63f9d0 Compare February 13, 2025 15:48
@github-actions github-actions bot added binding/python Issues for the Python package and removed binding/python Issues for the Python package labels Feb 13, 2025
@hntd187
Copy link
Collaborator

hntd187 commented Feb 13, 2025

It's not clear to me why you would want disable the conflict checker. I don't have any issues with this PR, I'm more curious what the use case is.

@daphnenhuch-at
Copy link
Contributor Author

It's not clear to me why you would want disable the conflict checker. I don't have any issues with this PR, I'm more curious what the use case is.

Our usage wants to disable any concurrent writes and that requires disabling the conflict checker and just erroring any time a conflicting/concurrent write occurs

@github-actions github-actions bot added the binding/python Issues for the Python package label Feb 13, 2025
@github-actions github-actions bot removed the binding/python Issues for the Python package label Feb 13, 2025
@daphnenhuch-at
Copy link
Contributor Author

This is good to go, could someone stamp it so that it's merge ready?

Copy link
Collaborator

@ion-elgreco ion-elgreco left a comment

Choose a reason for hiding this comment

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

Thanks!

@daphnenhuch-at
Copy link
Contributor Author

@ion-elgreco When is the next official delta lake release scheduled for?

@ion-elgreco
Copy link
Collaborator

@ion-elgreco When is the next official delta lake release scheduled for?

I hope to get the next python release out once my open PRs are merged

@daphnenhuch-at
Copy link
Contributor Author

@ion-elgreco When is the next official delta lake release scheduled for?

I hope to get the next python release out once my open PRs are merged

What about a rust release?

@ion-elgreco ion-elgreco added this pull request to the merge queue Feb 14, 2025
@ion-elgreco
Copy link
Collaborator

@ion-elgreco When is the next official delta lake release scheduled for?

I hope to get the next python release out once my open PRs are merged

What about a rust release?

Not sure, @rtyler thoughts?

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

Successfully merging this pull request may close these issues.

Delta Version 0.24 no longer allows disabling the conflict checker
3 participants