Skip to content

Commit learns how to rebase in case of conflict#920

Merged
paraseba merged 1 commit intomainfrom
push-mqryuzrskytr
Apr 18, 2025
Merged

Commit learns how to rebase in case of conflict#920
paraseba merged 1 commit intomainfrom
push-mqryuzrskytr

Conversation

@paraseba
Copy link
Copy Markdown
Collaborator

@paraseba paraseba commented Apr 18, 2025

Session.commit accepts new arguments:

rebase_with: ConflictSolver | None = None,
rebase_tries: int = 5

It uses the ConflictSolver to call Session.rebase in a loop, up to rebase_tries times, and try to commit after each rebase.

Closes: #919

@paraseba paraseba requested review from mpiannucci and rabernat April 18, 2025 17:37
Copy link
Copy Markdown
Contributor

@rabernat rabernat left a comment

Choose a reason for hiding this comment

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

API looks good! But I would massively increase the number of retries for the default.

I recommend updating docs as part of this PR if possible.

Comment thread icechunk-python/python/icechunk/_icechunk_python.pyi Outdated
Comment thread icechunk-python/python/icechunk/session.py Outdated
Comment thread icechunk-python/src/session.rs Outdated
@paraseba paraseba force-pushed the push-mqryuzrskytr branch from 55ddaa4 to 56f7b11 Compare April 18, 2025 17:55
rebase_tries.unwrap_or(1_000),
message,
metadata,
|_| async {},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Im guessing these are left for future work in python?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm currently using them for Rust tests, if they become useful we can expose them to python.

`Session.commit` accepts new arguments:

```
rebase_with: ConflictSolver | None = None,
rebase_tries: int = 5
```

It uses the `ConflictSolver` to call `Session.rebase` in a loop, up to
`rebase_tries` times, and try to commit after each rebase.
@paraseba paraseba force-pushed the push-mqryuzrskytr branch from 56f7b11 to 8c1b7be Compare April 18, 2025 17:59
@paraseba
Copy link
Copy Markdown
Collaborator Author

Default retries and documentation updated.

@paraseba paraseba merged commit a443f72 into main Apr 18, 2025
8 checks passed
@paraseba paraseba deleted the push-mqryuzrskytr branch April 18, 2025 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

API for automatic rebasing on commit

3 participants