Skip to content

Commit e04f149

Browse files
authored
fix(dvcfs): rename argument url to repo (#10754)
See #10700 for the motivation.
1 parent 488c30d commit e04f149

File tree

5 files changed

+65
-14
lines changed

5 files changed

+65
-14
lines changed

dvc/dependency/repo.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ def _make_fs(
154154
config["cache"]["dir"] = self.repo.cache.local_cache_dir
155155

156156
return DVCFileSystem(
157-
url=self.def_repo[self.PARAM_URL],
157+
repo=self.def_repo[self.PARAM_URL],
158158
rev=rev or self._get_rev(locked=locked),
159159
subrepos=True,
160160
config=config,

dvc/fs/dvc.py

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,10 @@ class _DVCFileSystem(AbstractFileSystem):
8383
cachable = False
8484
root_marker = "/"
8585

86-
def __init__( # noqa: PLR0913
86+
def __init__(
8787
self,
88-
url: Optional[str] = None,
88+
repo: Union["Repo", os.PathLike[str], str, None] = None,
8989
rev: Optional[str] = None,
90-
repo: Optional["Repo"] = None,
9190
subrepos: bool = False,
9291
repo_factory: Optional[RepoFactory] = None,
9392
fo: Optional[str] = None,
@@ -101,7 +100,8 @@ def __init__( # noqa: PLR0913
101100
"""DVC + git-tracked files fs.
102101
103102
Args:
104-
path (str, optional): URL or path to a DVC/Git repository.
103+
repo (str | os.PathLike[str] | Repo, optional): A url or a path to a DVC/Git
104+
repository, or a `Repo` instance.
105105
Defaults to a DVC repository in the current working directory.
106106
Both HTTP and SSH protocols are supported for remote Git repos
107107
(e.g. [user@]server:project.git).
@@ -111,7 +111,6 @@ def __init__( # noqa: PLR0913
111111
In case of a local repository, if rev is unspecified, it will
112112
default to the working directory.
113113
If the repo is not a Git repo, this option is ignored.
114-
repo (:obj:`Repo`, optional): `Repo` instance.
115114
subrepos (bool): traverse to subrepos.
116115
By default, it ignores subrepos.
117116
repo_factory (callable): A function to initialize subrepo with.
@@ -136,13 +135,23 @@ def __init__( # noqa: PLR0913
136135
... rev="main",
137136
... )
138137
"""
138+
from dvc.repo import Repo
139+
140+
# kwargs.get("url") is for maintaining backward compatibility
141+
repo = repo or fo or kwargs.get("url")
142+
if isinstance(repo, Repo):
143+
self._repo: Optional[Repo] = repo
144+
url = None
145+
else:
146+
self._repo = None
147+
url = os.fspath(repo) if repo else None
148+
139149
super().__init__()
140-
self._repo = repo
141150
self._repo_factory = repo_factory
142151
self._traverse_subrepos = subrepos
143152
self._repo_stack = ExitStack()
144153
self._repo_kwargs = {
145-
"url": url if url is not None else fo,
154+
"url": url,
146155
"rev": rev,
147156
"subrepos": subrepos,
148157
"config": config,

dvc/testing/api_tests.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,13 @@ def test_open(self, tmp_dir, dvc, remote):
3232
"fs_kwargs",
3333
[
3434
{},
35-
{"url": "{path}"},
36-
{"url": "{path}", "rev": "{default_branch}"},
37-
{"url": "file://{posixpath}"},
38-
{"url": "file://{posixpath}", "rev": "{default_branch}"},
35+
{"repo": "{path}"},
36+
{"repo": "{path}", "rev": "{default_branch}"},
37+
{"repo": "file://{posixpath}"},
38+
{"repo": "file://{posixpath}", "rev": "{default_branch}"},
39+
{"url": "{path}"}, # test for backward compatibility
3940
],
40-
ids=["current", "local", "local_rev", "git", "git_rev"],
41+
ids=["current", "local", "local_rev", "git", "git_rev", "local_url"],
4142
)
4243
def test_filesystem(
4344
self,
@@ -59,6 +60,8 @@ def test_filesystem(
5960
if clear_cache:
6061
remove(dvc.cache.repo.path)
6162

63+
if repo := fs_kwargs.get("repo"):
64+
fs_kwargs["repo"] = repo.format(path=tmp_dir, posixpath=tmp_dir.as_posix())
6265
if url := fs_kwargs.get("url"):
6366
fs_kwargs["url"] = url.format(path=tmp_dir, posixpath=tmp_dir.as_posix())
6467
if rev := fs_kwargs.get("rev"):

tests/unit/fs/test_dvc.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,3 +696,20 @@ def test_fsid_url(erepo_dir):
696696
fs = DVCFileSystem(repo=dvc)
697697
assert fs.fsid != old_fsid
698698
assert fs.fsid == "dvcfs_" + tokenize(url, erepo_dir.scm.get_rev())
699+
700+
701+
@pytest.mark.parametrize(
702+
"fs_kwargs",
703+
[
704+
lambda tmp_dir, dvc: {}, # noqa: ARG005
705+
lambda tmp_dir, dvc: {"repo": tmp_dir}, # noqa: ARG005
706+
lambda tmp_dir, dvc: {"repo": os.fspath(tmp_dir)}, # noqa: ARG005
707+
lambda tmp_dir, dvc: {"url": tmp_dir}, # noqa: ARG005
708+
lambda tmp_dir, dvc: {"url": os.fspath(tmp_dir)}, # noqa: ARG005
709+
lambda tmp_dir, dvc: {"repo": dvc}, # noqa: ARG005
710+
],
711+
)
712+
def test_init_arg(tmp_dir, dvc, fs_kwargs):
713+
fs = DVCFileSystem(**fs_kwargs(tmp_dir, dvc))
714+
715+
assert fs.repo.root_dir == dvc.root_dir

tests/unit/fs/test_dvcfs.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,7 @@ def test_maxdepth(tmp_dir, dvc, scm):
779779
commit="add dir",
780780
)
781781

782-
fs = DVCFileSystem(url=tmp_dir)
782+
fs = DVCFileSystem(tmp_dir)
783783
fs.get("dir", "dir1", recursive=True, maxdepth=1)
784784
assert (tmp_dir / "dir1").read_text() == {"file1": "file1"}
785785

@@ -803,3 +803,25 @@ def test_maxdepth(tmp_dir, dvc, scm):
803803
"subdir2": {"file3": "file3", "subdir3": {"file4": "file4"}},
804804
},
805805
}
806+
807+
808+
@pytest.mark.parametrize(
809+
"fs_args",
810+
[
811+
lambda tmp_dir, dvc: ((), {}), # noqa: ARG005
812+
lambda tmp_dir, dvc: ((dvc,), {}), # noqa: ARG005
813+
lambda tmp_dir, dvc: ((tmp_dir,), {}), # noqa: ARG005
814+
lambda tmp_dir, dvc: ((str(tmp_dir),), {}), # noqa: ARG005
815+
lambda tmp_dir, dvc: ((), {"repo": tmp_dir}), # noqa: ARG005
816+
lambda tmp_dir, dvc: ((), {"repo": os.fspath(tmp_dir)}), # noqa: ARG005
817+
# url= is deprecated, but is still supported for backward compatibility
818+
lambda tmp_dir, dvc: ((), {"url": tmp_dir}), # noqa: ARG005
819+
lambda tmp_dir, dvc: ((), {"url": os.fspath(tmp_dir)}), # noqa: ARG005
820+
lambda tmp_dir, dvc: ((), {"repo": dvc}), # noqa: ARG005
821+
],
822+
)
823+
def test_init_arg(tmp_dir, dvc, fs_args):
824+
args, kwargs = fs_args(tmp_dir, dvc)
825+
fs = DVCFileSystem(*args, **kwargs)
826+
827+
assert fs.repo.root_dir == dvc.root_dir

0 commit comments

Comments
 (0)