Skip to content

Commit

Permalink
Return created commits on merge and revert in transaction
Browse files Browse the repository at this point in the history
This seems like the more natural thing to do. Adjusts the tests to
expect the new behavior.

Also, since the merge logic only optionally creates a new commit on the
destination branch, we can unconditionally return the destination HEAD in
`LakeFSTransaction.merge()`.
  • Loading branch information
nicholasjng committed Jan 27, 2024
1 parent 2b8b645 commit b19118e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 15 deletions.
19 changes: 12 additions & 7 deletions src/lakefs_spec/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def commit(self, message: str, metadata: dict[str, str] | None = None) -> Refere

return self.branch.commit(message, metadata=metadata)

def merge(self, source_ref: str | Branch, into: str | Branch) -> str:
def merge(self, source_ref: str | Branch, into: str | Branch) -> Commit:
"""
Merge a branch into another branch in a repository.
Expand All @@ -166,17 +166,17 @@ def merge(self, source_ref: str | Branch, into: str | Branch) -> str:
Returns
-------
str
The ID of either the created merge commit, or the tip of the target branch.
Commit
Either the created merge commit, or the head commit of the target branch.
"""
source = ensurebranch(source_ref, self.repository, self.fs.client)
dest = ensurebranch(into, self.repository, self.fs.client)

if any(dest.diff(source)):
return source.merge_into(dest)
return dest.head.get_commit().id
source.merge_into(dest)
return dest.head.get_commit()

def revert(self, branch: str | Branch, ref: ReferenceType, parent_number: int = 1) -> None:
def revert(self, branch: str | Branch, ref: ReferenceType, parent_number: int = 1) -> Commit:
"""
Revert a previous commit on a branch.
Expand All @@ -190,13 +190,18 @@ def revert(self, branch: str | Branch, ref: ReferenceType, parent_number: int =
If there are multiple parents to a commit, specify to which parent
the commit should be reverted. ``parent_number = 1`` (the default)
refers to the first parent commit of the current ``branch`` tip.
Returns
-------
Commit
The created revert commit.
"""

b = ensurebranch(branch, self.repository, self.fs.client)

ref_id = ref if isinstance(ref, str) else ref.id
b.revert(ref_id, parent_number=parent_number)
return None
return b.head.get_commit()

def rev_parse(self, ref: ReferenceType) -> Commit:
"""
Expand Down
13 changes: 5 additions & 8 deletions tests/test_transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,16 @@ def test_transaction_revert(
random_file = random_file_factory.make()

lpath = str(random_file)
rpath = f"{repository.id}/{temp_branch.id}/{random_file.name}"

message = f"Add file {random_file.name}"

with fs.transaction(repository, temp_branch) as tx:
with fs.transaction(repository, temp_branch, automerge=True) as tx:
fs.put_file(lpath, f"{repository.id}/{tx.branch.id}/{random_file.name}")
tx.commit(message=message)
tx.revert(temp_branch, temp_branch.head)
revert_commit = tx.revert(temp_branch, temp_branch.head)

head, head_tilde = list(temp_branch.log(max_amount=2))
assert head.message.startswith("Merge")
assert head_tilde.message.startswith("Revert")
# first commit should be the merge commit
assert temp_branch.head.get_commit().message.startswith("Merge")
assert revert_commit.message.startswith("Revert")


def test_transaction_failure(
Expand All @@ -109,7 +107,6 @@ def test_transaction_failure(

lpath = str(random_file)
rpath = f"{repository.id}/{temp_branch.id}/{random_file.name}"

message = f"Add file {random_file.name}"

try:
Expand Down

0 comments on commit b19118e

Please sign in to comment.