Fetch storage_id once per Repository

Makefile

@@ -54,7 +54,7 @@ GIT_REF=$(shell git rev-parse --short HEAD --)
 REVISION=$(GIT_REF)$(DIRTY)
 export REVISION
 
-.PHONY: all clean esti lint test gen help
+.PHONY: all clean esti lint *-lint test gen help
 all: build
 
 clean:
@@ -160,9 +160,9 @@ build-binaries:
 	$(GOBUILD) -o $(LAKEFS_BINARY_NAME) -ldflags $(LD_FLAGS) -v ./cmd/$(LAKEFS_BINARY_NAME)
 	$(GOBUILD) -o $(LAKECTL_BINARY_NAME) -ldflags $(LD_FLAGS) -v ./cmd/$(LAKECTL_BINARY_NAME)
 
-lint: ## Lint code
-	$(GOCMD) tool golangci-lint run ./... $(GOLANGCI_LINT_FLAGS)
-	cd $(UI_DIR) && npm run lint
+lint: go-lint webui-lint ## Lint code
+
+lint-full: python-wrapper-lint lint ## Lint all code (slower)
 
 esti: ## run esti (system testing)
 	$(GOTEST) -v ./esti --args --system-tests
@@ -259,9 +259,15 @@ checks-validator: lint validate-proto validate-ui-format \
 	validate-api \
 	validate-wrapper validate-wrapgen-testcode
 
+go-lint:
+	$(GOCMD) tool golangci-lint run ./... $(GOLANGCI_LINT_FLAGS)
+
 python-wrapper-lint:
 	$(DOCKER) run --user $(UID_GID) --rm -v $(shell pwd):/mnt -e HOME=/tmp/ -w /mnt/clients/python-wrapper $(PYTHON_IMAGE) /bin/bash -c "./lint.sh"
 
+webui-lint:
+	cd $(UI_DIR) && npm run lint
+
 python-wrapper-gen-docs:
 	@version=$$(python -m pip install -r clients/python-wrapper/requirements.txt && python clients/python-wrapper/_version.py)
 	sphinx-build -b html -W clients/python-wrapper/docs clients/python-wrapper/_site/

clients/python-wrapper/README.md

@@ -34,11 +34,11 @@ from lakefs.client import Client
 
 # Using default client will attempt to authenticate with lakeFS server using configured credentials
 # If environment variables or .lakectl.yaml file exist 
-repo = lakefs.repository(repository_id="my-repo")
+repo = lakefs.repository(repo="my-repo")
 
 # Or explicitly initialize and provide a Client object 
 clt = Client(username="<lakefs_access_key_id>", password="<lakefs_secret_access_key>", host="<lakefs_endpoint>")
-repo = lakefs.Repository(repository_id="my-repo", client=clt)
+repo = lakefs.Repository(repo="my-repo", client=clt)
 
 # From this point, proceed using the package according to documentation
 main_branch = repo.create(storage_namespace="<storage_namespace>").branch(branch_id="main")

clients/python-wrapper/lakefs/_decorators.py

@@ -0,0 +1,28 @@
+"""
+Module for renaming kwargs
+
+Based on https://stackoverflow.com/a/74564394 but does not warn.
+"""
+
+from functools import wraps
+
+
+def renamed_parameters(**kwarg_map):
+    """
+    Decorator for renaming args
+
+    To rename old1 -> new1 and old2 -> new2, decorate the function definition:
+
+        @renamed_parameters(old1="new1", old2="new2"})
+        def func(new1, new2):
+          pass
+    """
+    def decorator(func):
+        @wraps(func)
+        def wrapped(*args, **kwargs):
+            new_kwargs = {}
+            for k, v in kwargs.items():
+                new_kwargs[kwarg_map.get(k, k)] = v
+            return func(*args, **new_kwargs)
+        return wrapped
+    return decorator

clients/python-wrapper/lakefs/branch.py

@@ -7,14 +7,19 @@
 import uuid
 import warnings
 from contextlib import contextmanager
-from typing import Optional, Generator, Iterable, Iterator, Literal, Dict
+from typing import Optional, Generator, Iterable, Iterator, Literal, Dict, TYPE_CHECKING
+
+from lakefs._decorators import renamed_parameters
 
 import lakefs_sdk
 from lakefs.client import Client
+from lakefs.getter import repository_for
 from lakefs.object import WriteableObject
 from lakefs.object import StoredObject
 from lakefs.import_manager import ImportManager
 from lakefs.reference import Reference, ReferenceType, generate_listing
+if TYPE_CHECKING:
+    from lakefs.repository import Repository
 from lakefs.models import Change, Commit
 from lakefs.tag import Tag
 from lakefs.exceptions import (
@@ -43,7 +48,7 @@ def object(self, path: str) -> WriteableObject:
         :param path: The object's path
         """
 
-        return WriteableObject(self.repo_id, self._id, path, client=self._client)
+        return WriteableObject(self._repo, self._id, path, client=self._client)
 
     def uncommitted(self, max_amount: Optional[int] = None, after: Optional[str] = None, prefix: Optional[str] = None,
                     **kwargs) -> Generator[Change]:
@@ -60,7 +65,7 @@ def uncommitted(self, max_amount: Optional[int] = None, after: Optional[str] = N
         """
 
         for diff in generate_listing(self._client.sdk_client.branches_api.diff_branch,
-                                     self._repo_id, self._id, max_amount=max_amount, after=after, prefix=prefix,
+                                     self.repo_id, self._id, max_amount=max_amount, after=after, prefix=prefix,
                                      **kwargs):
             yield Change(**diff.dict())
 
@@ -107,7 +112,7 @@ def delete_objects(self, object_paths: str | StoredObject | Iterable[str | Store
             object_paths = [o.path if isinstance(o, StoredObject) else o for o in object_paths]
         with api_exception_handler():
             return self._client.sdk_client.objects_api.delete_objects(
-                self._repo_id,
+                self.repo_id,
                 self._id,
                 lakefs_sdk.PathList(paths=object_paths)
             )
@@ -127,16 +132,17 @@ def reset_changes(self, path_type: Literal["common_prefix", "object", "reset"] =
         """
 
         reset_creation = lakefs_sdk.ResetCreation(path=path, type=path_type)
-        return self._client.sdk_client.branches_api.reset_branch(self._repo_id, self.id, reset_creation, **kwargs)
+        return self._client.sdk_client.branches_api.reset_branch(self.repo_id, self.id, reset_creation, **kwargs)
 
 
 class Branch(_BaseBranch):
     """
     Class representing a branch in lakeFS.
     """
 
-    def __init__(self, repository_id: str, branch_id: str, client: Optional[Client] = None):
-        super().__init__(repository_id, reference_id=branch_id, client=client)
+    @renamed_parameters(repository_id="repo")
+    def __init__(self, repo: Repository | str, branch_id: str, client: Optional[Client] = None):
+        super().__init__(repo, reference_id=branch_id, client=client)
 
     def get_commit(self):
         """
@@ -162,7 +168,7 @@ def cherry_pick(self, reference: ReferenceType, parent_number: Optional[int] = N
         cherry_pick_creation = lakefs_sdk.CherryPickCreation(ref=ref, parent_number=parent_number)
         with api_exception_handler():
             res = self._client.sdk_client.branches_api.cherry_pick(
-                self._repo_id, self._id, cherry_pick_creation, **kwargs)
+                self.repo_id, self._id, cherry_pick_creation, **kwargs)
             return Commit(**res.dict())
 
     def create(self, source_reference: ReferenceType, exist_ok: bool = False, **kwargs) -> Branch:
@@ -195,7 +201,7 @@ def handle_conflict(e: LakeFSException):
         reference_id = source_reference if isinstance(source_reference, str) else source_reference.id
         branch_creation = lakefs_sdk.BranchCreation(name=self._id, source=reference_id, **kwargs)
         with api_exception_handler(handle_conflict):
-            self._client.sdk_client.branches_api.create_branch(self._repo_id, branch_creation)
+            self._client.sdk_client.branches_api.create_branch(self.repo_id, branch_creation)
         return self
 
     @property
@@ -209,8 +215,8 @@ def head(self) -> Reference:
         :raise ServerException: for any other errors
         """
         with api_exception_handler():
-            branch = self._client.sdk_client.branches_api.get_branch(self._repo_id, self._id)
-        return Reference(self._repo_id, branch.commit_id, self._client)
+            branch = self._client.sdk_client.branches_api.get_branch(self.repo_id, self._id)
+        return Reference(self._repo, branch.commit_id, self._client)
 
     def commit(self, message: str, metadata: Optional[dict] = None, **kwargs) -> Reference:
         """
@@ -228,8 +234,8 @@ def commit(self, message: str, metadata: Optional[dict] = None, **kwargs) -> Ref
         commits_creation = lakefs_sdk.CommitCreation(message=message, metadata=metadata, **kwargs)
 
         with api_exception_handler():
-            c = self._client.sdk_client.commits_api.commit(self._repo_id, self._id, commits_creation)
-        return Reference(self._repo_id, c.id, self._client)
+            c = self._client.sdk_client.commits_api.commit(self.repo_id, self._id, commits_creation)
+        return Reference(self._repo, c.id, self._client)
 
     def delete(self, **kwargs) -> None:
         """
@@ -242,7 +248,7 @@ def delete(self, **kwargs) -> None:
         :raise ServerException: for any other errors
         """
         with api_exception_handler():
-            return self._client.sdk_client.branches_api.delete_branch(self._repo_id, self._id, **kwargs)
+            return self._client.sdk_client.branches_api.delete_branch(self.repo_id, self._id, **kwargs)
 
     def revert(self, reference: Optional[ReferenceType], parent_number: int = 0, *,
                reference_id: Optional[str] = None, **kwargs) -> Commit:
@@ -284,12 +290,12 @@ def revert(self, reference: Optional[ReferenceType], parent_number: int = 0, *,
 
         with api_exception_handler():
             self._client.sdk_client.branches_api.revert_branch(
-                self._repo_id,
+                self.repo_id,
                 self._id,
                 lakefs_sdk.RevertCreation(ref=ref, parent_number=parent_number, allow_empty=False),
                 **kwargs
             )
-            commit = self._client.sdk_client.commits_api.get_commit(self._repo_id, self._id)
+            commit = self._client.sdk_client.commits_api.get_commit(self.repo_id, self._id)
             return Commit(**commit.dict())
 
     def import_data(self, commit_message: str = "", metadata: Optional[dict] = None) -> ImportManager:
@@ -301,7 +307,7 @@ def import_data(self, commit_message: str = "", metadata: Optional[dict] = None)
             message is provided, uses the default server commit message for imports.
         :return: an ImportManager object
         """
-        return ImportManager(self._repo_id, self._id, commit_message, metadata, self._client)
+        return ImportManager(self.repo_id, self._id, commit_message, metadata, self._client)
 
     @contextmanager
     def transact(self, commit_message: str = "", commit_metadata: Optional[Dict] = None,
@@ -342,7 +348,7 @@ def transact(self, commit_message: str = "", commit_metadata: Optional[Dict] = N
         :param tag: Optional tag name to create after the transaction is successfully completed
         :return: a Transaction object to perform the operations on
         """
-        with Transaction(self._repo_id, self._id, commit_message, commit_metadata, delete_branch_on_error,
+        with Transaction(self._repo, self._id, commit_message, commit_metadata, delete_branch_on_error,
                          self._client, tag) as tx:
             yield tx
 
@@ -352,16 +358,17 @@ class _Transaction(_BaseBranch):
     def _get_tx_name() -> str:
         return f"tx-{uuid.uuid4()}"  # Don't rely on source branch name as this might exceed valid branch length
 
-    def __init__(self, repository_id: str, branch_id: str, commit_message: str = "",
+    @renamed_parameters(repository_id="repo")
+    def __init__(self, repo: Repository | str, branch_id: str, commit_message: str = "",
                  commit_metadata: Optional[Dict] = None, client: Optional[Client] = None, tag: Optional[str] = None):
         self._commit_message = commit_message
         self._commit_metadata = commit_metadata
         self._source_branch = branch_id
         self._tag = tag
 
         tx_name = self._get_tx_name()
-        self._tx_branch = Branch(repository_id, tx_name, client).create(branch_id, hidden=True)
-        super().__init__(repository_id, tx_name, client)
+        self._tx_branch = Branch(repo, tx_name, client).create(branch_id, hidden=True)
+        super().__init__(repo, tx_name, client)
 
     @property
     def source_id(self) -> str:
@@ -424,24 +431,33 @@ class Transaction:
     operations on the branch which will later be merged back into the source branch.
     Currently, transaction is supported only as a context manager.
     """
-
-    def __init__(self, repository_id: str, branch_id: str, commit_message: str = "",
+    _repo: Repository
+    _commit_message: str
+    _commit_metadata: Optional[Dict]
+    _source_branch: str
+    _tx: Optional[_Transaction]
+    _tx_branch: Optional[Branch]
+    _cleanup_branch: bool
+    _tag: Optional[str]
+
+    @renamed_parameters(repository_id="repo")
+    def __init__(self, repo: Repository | str, branch_id: str, commit_message: str = "",
                  commit_metadata: Optional[Dict] = None, delete_branch_on_error: bool = True,
-                 client: Optional[Client] = None, tag: Optional[str] = None):
-        self._repo_id = repository_id
+                 client: Optional[Client] = None, tag: Optional[str] = None) -> None:
+        self._repo = repository_for(repo, client)
         self._commit_message = commit_message
         self._commit_metadata = commit_metadata
         self._source_branch = branch_id
         self._client = client
-        self._tx: Optional["_Transaction"] = None
-        self._tx_branch: Optional["Branch"] = None
+        self._tx: Optional[_Transaction] = None
+        self._tx_branch: Optional[Branch] = None
         self._cleanup_branch = delete_branch_on_error
         self._tag = tag
 
     def __enter__(self):
-        self._tx = _Transaction(self._repo_id, self._source_branch, self._commit_message, self._commit_metadata,
+        self._tx = _Transaction(self._repo, self._source_branch, self._commit_message, self._commit_metadata,
                                 self._client, self._tag)
-        self._tx_branch = Branch(self._repo_id, self._tx.id, self._client)
+        self._tx_branch = Branch(self._repo, self._tx.id, self._client)
         return self._tx
 
     def __exit__(self, typ, value, traceback) -> Literal[False]:
@@ -463,6 +479,6 @@ def __exit__(self, typ, value, traceback) -> Literal[False]:
             raise TransactionException(f"Failed committing transaction {self._tx.id}: {e}") from e
 
         if self._tx.tag:
-            tag = Tag(self._repo_id, self._tx.tag, self._client)
+            tag = Tag(self._repo, self._tx.tag, self._client)
             tag.create(merge_commit_id)
         return False

clients/python-wrapper/lakefs/exceptions.py

@@ -194,3 +194,11 @@ def handle_http_error(resp: HTTPResponse) -> None:
         exc_class = _STATUS_CODE_TO_EXCEPTION.get(resp.status, ServerException)
         lakefs_ex = exc_class(resp.status, resp.reason, resp.data, headers_dict)
         raise lakefs_ex
+
+
+def io_exception_handler(e: LakeFSException):
+    if isinstance(e, NotFoundException):
+        return ObjectNotFoundException(e.status_code, e.reason)
+    if isinstance(e, (NotAuthorizedException, ForbiddenException)):
+        return PermissionException(e.status_code, e.reason)
+    return e

clients/python-wrapper/lakefs/getter.py

@@ -0,0 +1,23 @@
+"""
+Module containing str-or-Object getters
+
+This can help break import cycles.  To convert Repository | str to
+Repository in another client module you can
+
+from typing import TYPE_CHECKING
+if TYPE_CHECKING:
+    from repository import Repository
+from getter import repository_for
+
+repo: Repository = repository_for('abc')
+"""
+
+from __future__ import annotations
+from typing import Optional, TYPE_CHECKING
+from lakefs.client import Client
+# Import all of repository to break circular import.
+from lakefs import repository as repository_mod # Rename: repository is also a function.
+
+
+def repository_for(repo: repository_mod.Repository | str, client: Optional[Client]) -> repository_mod.Repository:
+    return repository_mod.Repository(repo, client) if isinstance(repo, str) else repo

clients/python-wrapper/lakefs/namedtuple.py

@@ -2,6 +2,8 @@
 LenientNamedTuple Module
 """
 
+from inspect import get_annotations
+
 
 class LenientNamedTuple:
     """
@@ -13,7 +15,7 @@ class LenientNamedTuple:
     unknown: dict = {}
 
     def __init__(self, **kwargs):
-        fields = list(self.__class__.__dict__["__annotations__"].keys())
+        fields = list(get_annotations(self.__class__).keys())
         for k, v in kwargs.items():
             if k in fields:
                 setattr(self, k, v)