Skip to content

Commit

Permalink
fix: show deprecation warnings from the Python wrapper by default (#7987
Browse files Browse the repository at this point in the history
)

* fix: show deprecation warnings from the Python wrapper by default

* fix tests

* fix lints

* rewrite docstring
  • Loading branch information
yonipeleg33 authored Jul 14, 2024
1 parent 3f9109f commit 3b9b498
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 8 deletions.
1 change: 1 addition & 0 deletions clients/python-wrapper/lakefs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from lakefs.tag import Tag
from lakefs.branch import Branch
from lakefs.object import StoredObject, WriteableObject, ObjectReader
from lakefs.branch import LakeFSDeprecationWarning


def repository(repository_id: str, *args, **kwargs) -> Repository:
Expand Down
15 changes: 10 additions & 5 deletions clients/python-wrapper/lakefs/branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

from __future__ import annotations

import sys
import uuid
import warnings
from contextlib import contextmanager
Expand All @@ -24,9 +23,15 @@
TransactionException
)

# Unless stated otherwise by passing `-W <something>` to Python, we display `DeprecationWarning`s by default.
if not sys.warnoptions:
warnings.simplefilter('always', DeprecationWarning)

class LakeFSDeprecationWarning(Warning):
"""
Warning about use of a deprecated lakeFS or client feature. Unlike
`DeprecationWarning`, this class is displayed by default. See
`default warning filter <https://docs.python.org/3/library/warnings.html#default-warning-filter>`_
for how to disable it.
"""


class _BaseBranch(Reference):

Expand Down Expand Up @@ -248,7 +253,7 @@ def revert(self, reference: Optional[ReferenceType], parent_number: int = 0, *,

if reference_id is not None:
warnings.warn(
"reference_id is deprecated, please use the `reference` argument.", DeprecationWarning
"reference_id is deprecated, please use the `reference` argument.", LakeFSDeprecationWarning
)
# 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.
Expand Down
7 changes: 4 additions & 3 deletions clients/python-wrapper/tests/utests/test_branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import lakefs_sdk
import pytest

import lakefs
from tests.utests.common import get_test_client, expect_exception_context
from lakefs.repository import Repository
from lakefs.exceptions import ConflictException
Expand Down Expand Up @@ -116,7 +117,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 @@ -149,7 +150,7 @@ def monkey_get_commit(repo_name, ref_name, **_):

expected_parent = 0
# reference_id passed, but not reference
with pytest.warns(DeprecationWarning, match="reference_id is deprecated.*"):
with pytest.warns(lakefs.LakeFSDeprecationWarning, match="reference_id is deprecated.*"):
branch.revert(None, reference_id=ref_id)

# neither reference nor reference_id passed
Expand All @@ -158,7 +159,7 @@ def monkey_get_commit(repo_name, ref_name, **_):

# both are passed, prefer ``reference_id``
with pytest.raises(ValueError, match="`reference_id` and `reference` both provided.*"), \
pytest.warns(DeprecationWarning, match="reference_id is deprecated.*"):
pytest.warns(lakefs.LakeFSDeprecationWarning, 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.
Expand Down

0 comments on commit 3b9b498

Please sign in to comment.