Fetch storage_id once per Repository by arielshaqed · Pull Request #10586 · treeverse/lakeFS

arielshaqed · 2026-11-01T14:19:07Z

Use Repository | str for all repository parameters, and cache storage_id on
that repository. Now it is fetched only once. Previously storage_id was
cached on each object - and each object would re-fetch storage_id.

To gain this advantage keep a Repository object and pass it through Refs,
Branches, Tags, etc. into your objects. You may still pass a repository_id,
of course, and forgo this advantage.

Code still supports the keyword argument repository_id - otherwise it would
break backwards compatibility. We may in future treat it as deprecated; for
now it is merely renamed using a decorator.

skshetry · 2026-11-02T05:28:28Z

Would it be simpler to keep the parameter name repository_id, widen its type to str | Repository? It's admittedly not the prettiest, but it would avoid the rename and re_arg while keeping the signatures (help(), mypy) unchanged.

arielshaqed · 2026-11-02T08:16:46Z

Would it be simpler to keep the parameter name repository_id, widen its type to str | Repository? It's admittedly not the prettiest, but it would avoid the rename and re_arg while keeping the signatures (help(), mypy) unchanged.

I seriously considered that, as you mentioned that change is much smaller. The thing is it would encourage users to pass a repo name rather than a repo object. But then they see no benefit - and continue making one get_repository API call per object used. APIs are forever (unfortunately).

skshetry · 2026-11-03T08:23:45Z

I seriously considered that, as you mentioned that change is much smaller. The thing is it would encourage users to pass a repo name rather than a repo object. But then they see no benefit - and continue making one get_repository API call per object used. APIs are forever (unfortunately).

Do we want to discourage repository_id altogether, though?

Right now re_arg is untyped, so type checkers (mypy and ty) see the decorated constructors as Any and stop checking calls to them.

We could type it with a ParamSpec, but that only exposes the real signature, so anyone still passing repository_id= would start getting type-checker errors.

help() already shows only repo either way, thanks to @wraps.

>>> from lakefs import StoredObject
>>> help(StoredObject.__init__)
__init__(
    self,
    repo: 'Repository | str',
    reference_id: 'str',
    path: 'str',
    client: 'Optional[Client]' = None
)

That seems fine if we're deprecating repository_id, but I wanted to check that's the intent.

Supporting both options is going to end up being more complex.

skshetry · 2026-11-03T09:08:11Z

+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:


A good alternative would be to import Repository lazily inside each functions that require it.

I would still need to define repository_for, though. So improving documentation to clear that up.

Use Repository | str for all repository parameters, and cache storage_id on that repository. Now it is fetched only once. Previously storage_id was cached on each object - and each object would re-fetch storage_id. To gain this advantage keep a Repository object and pass it through Refs, Branches, Tags, etc. into your objects. You may still pass a repository_id, of course, and forgo this advantage. Code still supports the keyword argument repository_id - otherwise it would break backwards compatibility. We may in future treat it as deprecated; for now it is merely renamed using a decorator.

This changed in Python 3.10. We use that, so follow [TFM](https://docs.python.org/3/howto/annotations.html#accessing-the-annotations-dict-of-an-object-in-python-3-10-and-newer)

arielshaqed

Thanks!

PTAL...

arielshaqed · 2026-11-04T09:52:21Z

+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:


I would still need to define repository_for, though. So improving documentation to clear that up.

skshetry · 2026-11-05T12:20:57Z

+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


Could we add type-hints to preserve type signature for all the decorated functions/constructors? Note that this will start raising typecheck errors for repository_id= usage.

Also, would love to hear your response for #10586 (comment) comment. :)

It would look something like follows:

from collections.abc import Callable from functools import wraps from typing import ParamSpec, TypeVar, cast P = ParamSpec("P") R = TypeVar("R") def renamed_parameters(**kwarg_map: str) -> Callable[[Callable[P, R]], Callable[P, R]]: """ 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: Callable[P, R]) -> Callable[P, R]: @wraps(func) def wrapped(*args: P.args, **kwargs: P.kwargs) -> R: new_kwargs = {} for k, v in kwargs.items(): new_kwargs[kwarg_map.get(k, k)] = v return cast(Callable[..., R], func)(*args, **new_kwargs) return wrapped return decorator

I do not want to start raising errors for repository_id... yet.

Also I am not sure what this actually changes. wrapped has the same type as func. Indeed, lsp-pyright gives the correct hint.

Unfortunately, mypy does not do type-inference, and loses those information.
See https://mypy.readthedocs.io/en/stable/generics.html#declaring-decorators.

# script.py from lakefs._decorators import renamed_parameters @renamed_parameters(repository_id="repo") def get_repo(repo: str) -> str: return repo get_repo(repo=123) get_repo(repository_id="my-repo")

With the above content:

$ mypy script.py Success: no issues found in 1 source file

$ pyright script.py pyright script.py /Users/saugat/projects/treeverse/lakeFS/clients/python-wrapper/script.py /Users/saugat/projects/treeverse/lakeFS/clients/python-wrapper/script.py:7:15 - error: Argument of type "Literal[123]" cannot be assigned to parameter "repo" of type "str" in function "get_repo" "Literal[123]" is not assignable to "str" (reportArgumentType) /Users/saugat/projects/treeverse/lakeFS/clients/python-wrapper/script.py:8:1 - error: Argument missing for parameter "repo" (reportCallIssue) /Users/saugat/projects/treeverse/lakeFS/clients/python-wrapper/script.py:8:10 - error: No parameter named "repository_id" (reportCallIssue) 3 errors, 0 warnings, 0 informations

$ ty check script.py All checks passed!

So, pyright reports a type-checking error even for repository_id. However, mypy and ty treat these decorated functions as untyped and skip type-checking them, meaning users lose the benefits of static type checking.

Note that if you do add typehints to the decorator, then all 3 of them will start raising typechecker errors for users using repository_id= keyword arguments (positional arguments will work fine).

…docs

arielshaqed

Thanks! Not sure I should change the decorator.

arielshaqed · 2026-11-05T12:46:12Z

+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


I do not want to start raising errors for repository_id... yet.

arielshaqed · 2026-11-05T12:50:21Z

+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


Also I am not sure what this actually changes. wrapped has the same type as func. Indeed, lsp-pyright gives the correct hint.

skshetry

LGTM, except of type-hints conversation, but not a blocker. 🙂

It's not useful there, but it _is_ more consistent.

- Improves type-checking of methods with renamed parameters. - Flags a type error on uses of repository_id. But the call itself works, of course!

Use overloads to mark this usage as _deprecated_, but allow it.

arielshaqed

I decided:

Add typing to the renaming.
Allow repository_id to pass type-checking.
Deprecate these uses.

It is slightly verbose because the Python type-checking language is garbage. But it means that I do not harm existing users.

PTAL!

github-actions Bot added area/testing Improvements or additions to tests area/UI Improvements or additions to UI infrastructure build, deploy and release processes area/sdk/python labels Nov 1, 2026

arielshaqed force-pushed the feature/py-hl-less-get-repo branch from 1d7f07f to a950323 Compare November 1, 2026 14:19

arielshaqed added the include-changelog PR description should be included in next release changelog label Nov 1, 2026

arielshaqed requested a review from skshetry November 1, 2026 14:22

arielshaqed force-pushed the feature/py-hl-less-get-repo branch from a950323 to 0c78d35 Compare November 1, 2026 14:49