Skip to content

Commit

Permalink
fix: fail revert operation in the Python wrapper in case both `refe…
Browse files Browse the repository at this point in the history
…rence` and `reference_id` args are provided (#7983)

* fix: fail `revert` operation in the Python wrapper in case both `reference` and `reference_id` args are provided

* review comments

* fix test
  • Loading branch information
yonipeleg33 authored Jul 11, 2024
1 parent a7faa87 commit 713668e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
5 changes: 5 additions & 0 deletions clients/python-wrapper/lakefs/branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,11 @@ def revert(self, reference: Optional[ReferenceType], parent_number: int = 0, *,
warnings.warn(
"reference_id is deprecated, please use the `reference` argument.", DeprecationWarning
)
# We show the error in case both are provided only after showing the deprecation warning, in order
# for the user to have the most contextual clarity.
if reference is not None:
raise ValueError("`reference_id` and `reference` both provided "
"Use only the `reference` argument.")

# Handle reference_id as a deprecated alias to reference.
reference = reference or reference_id
Expand Down
8 changes: 4 additions & 4 deletions clients/python-wrapper/tests/utests/test_branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def test_branch_revert(monkeypatch):
branch = get_test_branch()
ref_id = "ab1234"
expected_parent = 0
with monkeypatch.context():
with (monkeypatch.context()):
def monkey_revert_branch(repo_name, branch_name, revert_branch_creation, *_):
assert repo_name == branch.repo_id
assert branch_name == branch.id
Expand Down Expand Up @@ -157,9 +157,9 @@ def monkey_get_commit(repo_name, ref_name, **_):
branch.revert(None)

# both are passed, prefer ``reference_id``
with pytest.warns(DeprecationWarning, match="reference_id is deprecated.*"):
with pytest.raises(ValueError, match="`reference_id` and `reference` both provided.*"), \
pytest.warns(DeprecationWarning, match="reference_id is deprecated.*"):
# this is not a high-quality test, but it would throw if the revert API
# was called with reference "hello" due to the monkey-patching above
# always returning "ab1234" as ref ID.
c = branch.revert(ref_id, reference_id="hello")
assert c.id == ref_id
branch.revert(ref_id, reference_id="hello")

0 comments on commit 713668e

Please sign in to comment.