diff --git a/dvc/api/artifacts.py b/dvc/api/artifacts.py index 687fc86a8..7ac85bf64 100644 --- a/dvc/api/artifacts.py +++ b/dvc/api/artifacts.py @@ -1,3 +1,4 @@ +import os from typing import Any, Optional from dvc.repo import Repo @@ -36,12 +37,22 @@ def artifacts_show( if version and stage: raise ValueError("Artifact version and stage are mutually exclusive.") + from dvc.repo.artifacts import Artifacts + from dvc.utils import as_posix + repo_kwargs: dict[str, Any] = { "subrepos": True, "uninitialized": True, } + + dirname, _ = Artifacts.parse_path(name) with Repo.open(repo, **repo_kwargs) as _repo: rev = _repo.artifacts.get_rev(name, version=version, stage=stage) with _repo.switch(rev): - path = _repo.artifacts.get_path(name) - return {"rev": rev, "path": path} + root = _repo.fs.root_marker + _dirname = _repo.fs.join(root, dirname) if dirname else root + with Repo(_dirname, fs=_repo.fs, scm=_repo.scm) as r: + path = r.artifacts.get_path(name) + path = _repo.fs.join(_repo.fs.root_marker, as_posix(path)) + parts = _repo.fs.relparts(path, _repo.root_dir) + return {"rev": rev, "path": os.path.join(*parts)} diff --git a/dvc/repo/artifacts.py b/dvc/repo/artifacts.py index 3b0bc243e..bb6584498 100644 --- a/dvc/repo/artifacts.py +++ b/dvc/repo/artifacts.py @@ -12,7 +12,7 @@ from dvc.exceptions import ( InvalidArgumentError, ) from dvc.log import logger -from dvc.utils import relpath, resolve_output +from dvc.utils import as_posix, relpath, resolve_output from dvc.utils.objects import cached_property from dvc.utils.serialize import modify_yaml @@ -99,7 +99,7 @@ class Artifacts: """Read artifacts from dvc.yaml.""" artifacts: dict[str, dict[str, Artifact]] = {} for dvcfile, dvcfile_artifacts in self.repo.index._artifacts.items(): - dvcyaml = relpath(dvcfile, self.repo.root_dir) + dvcyaml = self.repo.fs.relpath(dvcfile, self.repo.root_dir) artifacts[dvcyaml] = {} for name, value in dvcfile_artifacts.items(): try: @@ -147,8 +147,8 @@ class Artifacts: gto_tags: list["GTOTag"] = sort_versions(parse_tag(tag) for tag in tags) return gto_tags[0].tag.target - def get_path(self, name: str): - """Return repo fspath for the given artifact.""" + @classmethod + def parse_path(cls, name: str) -> tuple[Optional[str], str]: from gto.constants import SEPARATOR_IN_NAME, fullname_re name = _reformat_name(name) @@ -158,13 +158,37 @@ class Artifacts: dirname = m.group("dirname") if dirname: dirname = dirname.rstrip(SEPARATOR_IN_NAME) - dvcyaml = os.path.join(dirname, PROJECT_FILE) if dirname else PROJECT_FILE - artifact_name = m.group("name") + + return dirname, m.group("name") + + def get_path(self, name: str): + """Return fspath for the given artifact relative to the git root.""" + from dvc.fs import GitFileSystem + + dirname, artifact_name = self.parse_path(name) + # `name`/`dirname` are expected to be a git root relative. + # We convert it to dvc-root relative path so that we can read artifacts + # from dvc.yaml file. + # But we return dirname intact, as we want to return a git-root relative path. + # This is useful when reading from `dvcfs` from remote. + fs = self.repo.fs + assert self.scm + if isinstance(fs, GitFileSystem): + scm_root = fs.root_marker + else: + scm_root = self.scm.root_dir + + dirparts = posixpath.normpath(dirname).split(posixpath.sep) if dirname else () + abspath = fs.join(scm_root, *dirparts, PROJECT_FILE) + rela = fs.relpath(abspath, self.repo.root_dir) try: - artifact = self.read()[dvcyaml][artifact_name] + artifact = self.read()[rela][artifact_name] except KeyError as exc: raise ArtifactNotFoundError(name) from exc - return os.path.join(dirname, artifact.path) if dirname else artifact.path + + path = posixpath.join(dirname or "", artifact.path) + parts = posixpath.normpath(path).split(posixpath.sep) + return os.path.join(*parts) def download( self, @@ -177,15 +201,28 @@ class Artifacts: ) -> tuple[int, str]: """Download the specified artifact.""" from dvc.fs import download as fs_download + from dvc.repo import Repo logger.debug("Trying to download artifact '%s' via DVC", name) rev = self.get_rev(name, version=version, stage=stage) + + dirname, _ = self.parse_path(name) with self.repo.switch(rev): - path = self.get_path(name) + root = self.repo.fs.root_marker + _dirname = self.repo.fs.join(root, dirname) if dirname else root + with Repo(_dirname, fs=self.repo.fs, scm=self.repo.scm) as r: + path = r.artifacts.get_path(name) + path = self.repo.fs.join(root, as_posix(path)) + path = self.repo.fs.relpath(path, self.repo.root_dir) + # when the `repo` is a subrepo, the path `/subrepo/myart.pkl` for dvcfs + # should be translated as `/myart.pkl`, + # i.e. relative to the root of the subrepo + path = self.repo.fs.join(root, path) + path = self.repo.fs.normpath(path) + out = resolve_output(path, out, force=force) fs = self.repo.dvcfs - fs_path = fs.from_os_path(path) - count = fs_download(fs, fs_path, os.path.abspath(out), jobs=jobs) + count = fs_download(fs, path, os.path.abspath(out), jobs=jobs) return count, out @staticmethod diff --git a/tests/func/api/test_artifacts.py b/tests/func/api/test_artifacts.py new file mode 100644 index 000000000..4907e9046 --- /dev/null +++ b/tests/func/api/test_artifacts.py @@ -0,0 +1,74 @@ +from os.path import join, normpath + +import pytest + +from dvc.api import artifacts_show +from dvc.testing.tmp_dir import make_subrepo +from dvc.utils import as_posix +from tests.func.artifacts.test_artifacts import get_tag_and_name, make_artifact + + +@pytest.mark.parametrize("sub", ["sub", ""]) +def test_artifacts_show(tmp_dir, dvc, scm, sub): + subdir = tmp_dir / sub + + dirname = str(subdir.relative_to(tmp_dir)) + tag, name = get_tag_and_name(as_posix(dirname), "myart", "v2.0.0") + make_artifact(tmp_dir, "myart", tag, subdir / "myart.pkl") + + assert artifacts_show(name) == { + "path": normpath(join(dirname, "myart.pkl")), + "rev": scm.get_rev(), + } + assert artifacts_show(name, repo=tmp_dir.fs_path) == { + "path": normpath(join(dirname, "myart.pkl")), + "rev": scm.get_rev(), + } + assert artifacts_show(name, repo=f"file://{tmp_dir.as_posix()}") == { + "path": normpath(join(dirname, "myart.pkl")), + "rev": scm.get_rev(), + } + + assert artifacts_show(name, repo=subdir.fs_path) == { + "path": normpath(join(dirname, "myart.pkl")), + "rev": scm.get_rev(), + } + with subdir.chdir(): + assert artifacts_show(name) == { + "path": normpath(join(dirname, "myart.pkl")), + "rev": scm.get_rev(), + } + + +@pytest.mark.parametrize("sub", ["sub", ""]) +def test_artifacts_show_subrepo(tmp_dir, scm, sub): + subrepo = tmp_dir / "subrepo" + make_subrepo(subrepo, scm) + subdir = subrepo / sub + + dirname = str(subdir.relative_to(tmp_dir)) + tag, name = get_tag_and_name(as_posix(dirname), "myart", "v2.0.0") + make_artifact(subrepo, "myart", tag, subdir / "myart.pkl") + + assert artifacts_show(name) == { + "path": join(dirname, "myart.pkl"), + "rev": scm.get_rev(), + } + assert artifacts_show(name, repo=tmp_dir.fs_path) == { + "path": join(dirname, "myart.pkl"), + "rev": scm.get_rev(), + } + assert artifacts_show(name, repo=f"file://{tmp_dir.as_posix()}") == { + "path": join(dirname, "myart.pkl"), + "rev": scm.get_rev(), + } + + assert artifacts_show(name, repo=subdir.fs_path) == { + "path": str((subdir / "myart.pkl").relative_to(subrepo)), + "rev": scm.get_rev(), + } + with subdir.chdir(): + assert artifacts_show(name) == { + "path": str((subdir / "myart.pkl").relative_to(subrepo)), + "rev": scm.get_rev(), + } diff --git a/tests/func/artifacts/test_artifacts.py b/tests/func/artifacts/test_artifacts.py index fcad4d307..12456b317 100644 --- a/tests/func/artifacts/test_artifacts.py +++ b/tests/func/artifacts/test_artifacts.py @@ -6,7 +6,9 @@ import pytest from dvc.annotations import Artifact from dvc.exceptions import ArtifactNotFoundError, InvalidArgumentError -from dvc.repo.artifacts import check_name_format +from dvc.repo.artifacts import Artifacts, check_name_format +from dvc.testing.tmp_dir import make_subrepo +from dvc.utils import as_posix from dvc.utils.strictyaml import YAMLSyntaxError, YAMLValidationError dvcyaml = { @@ -185,7 +187,7 @@ def test_get_rev(tmp_dir, dvc, scm): dvc.artifacts.get_rev("myart", stage="prod") -def test_get_path(tmp_dir, dvc): +def test_get_path(tmp_dir, dvc, scm): (tmp_dir / "dvc.yaml").dump(dvcyaml) subdir = tmp_dir / "subdir" subdir.mkdir() @@ -206,3 +208,76 @@ def test_parametrized(tmp_dir, dvc): assert tmp_dir.dvc.artifacts.read() == { "dvc.yaml": {"myart": Artifact(path="myart.pkl", type="model")} } + + +def test_get_path_subrepo(tmp_dir, scm, dvc): + subrepo = tmp_dir / "subrepo" + make_subrepo(subrepo, scm) + (subrepo / "dvc.yaml").dump(dvcyaml) + + assert dvc.artifacts.get_path("subrepo:myart") == os.path.join( + "subrepo", "myart.pkl" + ) + assert dvc.artifacts.get_path("subrepo/dvc.yaml:myart") == os.path.join( + "subrepo", "myart.pkl" + ) + + assert subrepo.dvc.artifacts.get_path("subrepo:myart") == os.path.join( + "subrepo", "myart.pkl" + ) + assert subrepo.dvc.artifacts.get_path("subrepo/dvc.yaml:myart") == os.path.join( + "subrepo", "myart.pkl" + ) + + +def get_tag_and_name(dirname, name, version): + tagname = f"{name}@{version}" + if dirname in (os.curdir, ""): + return tagname, name + return f"{dirname}={tagname}", f"{dirname}:{name}" + + +def make_artifact(tmp_dir, name, tag, path) -> Artifact: + artifact = Artifact(path=path.name, type="model") + dvcfile = path.with_name("dvc.yaml") + + tmp_dir.scm_gen(path, "hello_world", commit="add myart.pkl") + tmp_dir.dvc.artifacts.add(name, artifact, dvcfile=os.fspath(dvcfile)) + tmp_dir.scm.add_commit([dvcfile], message="add dvc.yaml") + tmp_dir.scm.tag(tag, annotated=True, message="foo") + return artifact + + +@pytest.mark.parametrize("sub", ["sub", ""]) +def test_artifacts_download(tmp_dir, dvc, scm, sub): + subdir = tmp_dir / sub + dirname = str(subdir.relative_to(tmp_dir)) + tag, name = get_tag_and_name(as_posix(dirname), "myart", "v2.0.0") + make_artifact(tmp_dir, "myart", tag, subdir / "myart.pkl") + + result = (1, "myart.pkl") + assert Artifacts.get(".", name, force=True) == result + assert Artifacts.get(tmp_dir.fs_path, name, force=True) == result + assert Artifacts.get(f"file://{tmp_dir.as_posix()}", name, force=True) == result + assert Artifacts.get(subdir.fs_path, name, force=True) == result + with subdir.chdir(): + assert Artifacts.get(".", name, force=True) == result + + +@pytest.mark.parametrize("sub", ["sub", ""]) +def test_artifacts_download_subrepo(tmp_dir, scm, sub): + subrepo = tmp_dir / "subrepo" + make_subrepo(subrepo, scm) + subdir = subrepo / sub + + dirname = str(subdir.relative_to(tmp_dir)) + tag, name = get_tag_and_name(as_posix(dirname), "myart", "v2.0.0") + make_artifact(subrepo, "myart", tag, subdir / "myart.pkl") + + result = (1, "myart.pkl") + assert Artifacts.get(".", name) == result + assert Artifacts.get(tmp_dir.fs_path, name, force=True) == result + assert Artifacts.get(f"file://{tmp_dir.as_posix()}", name, force=True) == result + assert Artifacts.get(subdir.fs_path, name, force=True) == result + with subdir.chdir(): + assert Artifacts.get(".", name, force=True) == result