Repository: iterative/dvc
Pull request #10303 (2024-02-15 08:16:51)

studio: add subdir to live metrics post messages to support live experiments in monorepos
* [x] ❗ I have followed the [Contributing to DVC](https://dvc.org/doc/user-guide/contributing/core) checklist.

* [x] 📖 If this PR requires [documentation](https://dvc.org/doc) updates, I have created a separate PR (or issue, at least) in [dvc.org](https://github.com/iterative/dvc.org) and linked it here.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

----

Part of https://github.com/iterative/studio/issues/8848

Depends on https://github.com/iterative/dvc-studio-client/pull/154 / https://github.com/iterative/studio/pull/9173

Required by https://github.com/iterative/dvclive/pull/787

Repeat of https://github.com/iterative/dvc/pull/10291 but stripped back to only send subdir.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/iterative/dvc/pull/10303?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iterative) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`5e2c441`)](https://app.codecov.io/gh/iterative/dvc/commit/5e2c441d28274a2cfbb4c4eec071024987143e80?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iterative) 90.44% compared to head [(`4c08ad8`)](https://app.codecov.io/gh/iterative/dvc/pull/10303?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iterative) 90.17%.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main   #10303      +/-   ##
==========================================
- Coverage   90.44%   90.17%   -0.28%     
==========================================
  Files         494      494              
  Lines       37821    37826       +5     
  Branches     5469     5473       +4     
==========================================
- Hits        34206    34108      -98     
- Misses       2983     3058      +75     
- Partials      632      660      +28     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/iterative/dvc/pull/10303?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iterative).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iterative).

mattseddon: Currently needs a Studio release
mattseddon: @dberenbaum can you PTAL at this now Studio has been released
dberenbaum: Should we add a test to https://github.com/iterative/dvc/blob/main/tests/integration/test_studio_live_experiments.py to ensure the subdir is sent to studio?
mattseddon: > Should we add a test to https://github.com/iterative/dvc/blob/main/tests/integration/test_studio_live_experiments.py to ensure the subdir is sent to studio?

I felt like the complexity that needed to be tested was addressed by `test_monorepo_relpath` but I have added `test_post_to_studio_subdir` (Hopefully it passes on Windows)

Patch:
diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py
index 36d0f5675..c10965f43 100644
--- a/dvc/repo/__init__.py
+++ b/dvc/repo/__init__.py
@@ -13,7 +13,6 @@ from dvc.exceptions import (
 )
 from dvc.ignore import DvcIgnoreFilter
 from dvc.log import logger
-from dvc.utils import as_posix
 from dvc.utils.objects import cached_property
 
 if TYPE_CHECKING:
@@ -351,16 +350,6 @@ class Repo:
         # fs.
         self._reset()
 
-    @property
-    def subrepo_relpath(self) -> str:
-        from dvc.fs import GitFileSystem
-
-        scm_root_dir = "/" if isinstance(self.fs, GitFileSystem) else self.scm.root_dir
-
-        relpath = as_posix(self.fs.relpath(self.root_dir, scm_root_dir))
-
-        return "" if relpath == "." else relpath
-
     @property
     def data_index(self) -> "DataIndex":
         from dvc_data.index import DataIndex
diff --git a/dvc/repo/experiments/executor/base.py b/dvc/repo/experiments/executor/base.py
index 22c7ee3d4..eda9abb83 100644
--- a/dvc/repo/experiments/executor/base.py
+++ b/dvc/repo/experiments/executor/base.py
@@ -23,7 +23,10 @@ from dvc.repo.params.show import _collect_top_level_params
 from dvc.stage.serialize import to_lockfile
 from dvc.utils import dict_sha256, env2bool, relpath
 from dvc.utils.fs import remove
-from dvc.utils.studio import env_to_config
+from dvc.utils.studio import (
+    env_to_config,
+    get_subrepo_relpath,
+)
 
 if TYPE_CHECKING:
     from queue import Queue
@@ -624,6 +627,7 @@ class BaseExecutor(ABC):
                     params=to_studio_params(dvc.params.show()),
                     dvc_studio_config=dvc_studio_config,
                     message=message,
+                    subdir=get_subrepo_relpath(dvc),
                 )
                 logger.debug("Running repro in '%s'", os.getcwd())
                 yield dvc
diff --git a/dvc/utils/studio.py b/dvc/utils/studio.py
index 45aa7b17e..470d19549 100644
--- a/dvc/utils/studio.py
+++ b/dvc/utils/studio.py
@@ -12,10 +12,14 @@ from dvc.env import (
     DVC_STUDIO_URL,
 )
 from dvc.log import logger
+from dvc.utils import as_posix
 
 if TYPE_CHECKING:
     from requests import Response
 
+    from dvc.repo import Repo
+
+
 logger = logger.getChild(__name__)
 
 STUDIO_URL = "https://studio.iterative.ai"
@@ -111,3 +115,13 @@ def env_to_config(env: dict[str, Any]) -> dict[str, Any]:
     if DVC_STUDIO_URL in env:
         config["url"] = env[DVC_STUDIO_URL]
     return config
+
+
+def get_subrepo_relpath(repo: "Repo") -> str:
+    from dvc.fs import GitFileSystem
+
+    scm_root_dir = "/" if isinstance(repo.fs, GitFileSystem) else repo.scm.root_dir
+
+    relpath = as_posix(repo.fs.relpath(repo.root_dir, scm_root_dir))
+
+    return "" if relpath == "." else relpath
diff --git a/pyproject.toml b/pyproject.toml
index d990f09a7..1904067a5 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -38,7 +38,7 @@ dependencies = [
     "dvc-data>=3.13,<3.14",
     "dvc-http>=2.29.0",
     "dvc-render>=1.0.1,<2",
-    "dvc-studio-client>=0.19,<1",
+    "dvc-studio-client>=0.20,<1",
     "dvc-task>=0.3.0,<1",
     "flatten_dict<1,>=0.4.1",
     # https://github.com/iterative/dvc/issues/9654


Test patch:
diff --git a/tests/integration/test_studio_live_experiments.py b/tests/integration/test_studio_live_experiments.py
index c56079f52..da1136f08 100644
--- a/tests/integration/test_studio_live_experiments.py
+++ b/tests/integration/test_studio_live_experiments.py
@@ -8,8 +8,12 @@ from dvc.env import (
     DVC_STUDIO_TOKEN,
     DVC_STUDIO_URL,
 )
+from dvc.repo import Repo
+from dvc.testing.scripts import COPY_SCRIPT
+from dvc.utils.studio import get_subrepo_relpath
 
 
+@pytest.mark.studio
 @pytest.mark.parametrize("tmp", [True, False])
 @pytest.mark.parametrize("offline", [True, False])
 def test_post_to_studio(
@@ -66,10 +70,45 @@ def test_post_to_studio(
         }
 
 
+@pytest.mark.studio
 @pytest.mark.parametrize("tmp", [True, False])
-def test_post_to_studio_custom_message(
-    tmp_dir, dvc, scm, exp_stage, mocker, monkeypatch, tmp
-):
+def test_post_to_studio_subdir(tmp_dir, scm, mocker, monkeypatch, tmp):
+    live_exp_subdir = "project_a"
+
+    tmp_dir.scm_gen(
+        {
+            live_exp_subdir: {
+                "params.yaml": "foo: 1",
+                "metrics.yaml": "foo: 1",
+                "copy.py": COPY_SCRIPT.encode("utf-8"),
+            },
+        },
+        commit="git init",
+    )
+
+    project_a_dvc = Repo.init(tmp_dir / live_exp_subdir, subdir=True)
+    with monkeypatch.context() as m:
+        m.chdir(project_a_dvc.root_dir)
+
+        exp_stage = project_a_dvc.run(
+            cmd="python copy.py params.yaml metrics.yaml",
+            metrics_no_cache=["metrics.yaml"],
+            params=["foo"],
+            name="copy-file",
+        )
+
+        scm.add(
+            [
+                ".gitignore",
+                "copy.py",
+                "dvc.lock",
+                "dvc.yaml",
+                "metrics.yaml",
+                "params.yaml",
+            ]
+        )
+        scm.commit("dvc init project_a")
+
     valid_response = mocker.MagicMock()
     valid_response.status_code = 200
     mocked_post = mocker.patch("requests.post", return_value=valid_response)
@@ -79,12 +118,16 @@ def test_post_to_studio_custom_message(
     monkeypatch.setenv(env.STUDIO_TOKEN, "STUDIO_TOKEN")
 
     baseline_sha = scm.get_rev()
-    exp_rev = first(
-        dvc.experiments.run(
-            exp_stage.addressing, params=["foo=1"], tmp_dir=tmp, message="foo"
+    with monkeypatch.context() as m:
+        m.chdir(project_a_dvc.root_dir)
+        exp_rev = first(
+            project_a_dvc.experiments.run(
+                exp_stage.addressing, params=["foo=24"], tmp_dir=tmp
+            )
         )
-    )
-    name = dvc.experiments.get_exact_name([exp_rev])[exp_rev]
+
+    name = project_a_dvc.experiments.get_exact_name([exp_rev])[exp_rev]
+    project_a_dvc.close()
     assert mocked_post.call_count == 2
 
     start_call = mocked_post.call_args_list[0]
@@ -94,7 +137,57 @@ def test_post_to_studio_custom_message(
         "repo_url": "STUDIO_REPO_URL",
         "baseline_sha": baseline_sha,
         "name": name,
-        "params": {"params.yaml": {"foo": 1}},
+        "params": {"params.yaml": {"foo": 24}},
+        "subdir": live_exp_subdir,
         "client": "dvc",
-        "message": "foo",
     }
+
+
+@pytest.mark.studio
+def test_monorepo_relpath(tmp_dir, scm):
+    from dvc.repo.destroy import destroy
+
+    tmp_dir.gen({"project_a": {}, "subdir/project_b": {}})
+
+    non_monorepo = Repo.init(tmp_dir)
+    assert get_subrepo_relpath(non_monorepo) == ""
+
+    destroy(non_monorepo)
+
+    monorepo_project_a = Repo.init(tmp_dir / "project_a", subdir=True)
+
+    assert get_subrepo_relpath(monorepo_project_a) == "project_a"
+
+    monorepo_project_b = Repo.init(tmp_dir / "subdir" / "project_b", subdir=True)
+
+    assert get_subrepo_relpath(monorepo_project_b) == "subdir/project_b"
+
+
+@pytest.mark.studio
+def test_virtual_monorepo_relpath(tmp_dir, scm):
+    from dvc.fs.git import GitFileSystem
+    from dvc.repo.destroy import destroy
+
+    tmp_dir.gen({"project_a": {}, "subdir/project_b": {}})
+    scm.commit("initial commit")
+    gfs = GitFileSystem(scm=scm, rev="master")
+
+    non_monorepo = Repo.init(tmp_dir)
+    non_monorepo.fs = gfs
+    non_monorepo.root_dir = "/"
+
+    assert get_subrepo_relpath(non_monorepo) == ""
+
+    destroy(non_monorepo)
+
+    monorepo_project_a = Repo.init(tmp_dir / "project_a", subdir=True)
+    monorepo_project_a.fs = gfs
+    monorepo_project_a.root_dir = "/project_a"
+
+    assert get_subrepo_relpath(monorepo_project_a) == "project_a"
+
+    monorepo_project_b = Repo.init(tmp_dir / "subdir" / "project_b", subdir=True)
+    monorepo_project_b.fs = gfs
+    monorepo_project_b.root_dir = "/subdir/project_b"
+
+    assert get_subrepo_relpath(monorepo_project_b) == "subdir/project_b"
diff --git a/tests/unit/repo/test_repo.py b/tests/unit/repo/test_repo.py
index c3ea4cd71..897a4c3b3 100644
--- a/tests/unit/repo/test_repo.py
+++ b/tests/unit/repo/test_repo.py
@@ -135,51 +135,3 @@ def test_dynamic_cache_initialization(tmp_dir, scm):
     dvc.close()
 
     Repo(str(tmp_dir)).close()
-
-
-def test_monorepo_relpath(tmp_dir, scm):
-    from dvc.repo.destroy import destroy
-
-    tmp_dir.gen({"project_a": {}, "subdir/project_b": {}})
-
-    non_monorepo = Repo.init(tmp_dir)
-    assert non_monorepo.subrepo_relpath == ""
-
-    destroy(non_monorepo)
-
-    monorepo_project_a = Repo.init(tmp_dir / "project_a", subdir=True)
-
-    assert monorepo_project_a.subrepo_relpath == "project_a"
-
-    monorepo_project_b = Repo.init(tmp_dir / "subdir" / "project_b", subdir=True)
-
-    assert monorepo_project_b.subrepo_relpath == "subdir/project_b"
-
-
-def test_virtual_monorepo_relpath(tmp_dir, scm):
-    from dvc.fs.git import GitFileSystem
-    from dvc.repo.destroy import destroy
-
-    tmp_dir.gen({"project_a": {}, "subdir/project_b": {}})
-    scm.commit("initial commit")
-    gfs = GitFileSystem(scm=scm, rev="master")
-
-    non_monorepo = Repo.init(tmp_dir)
-    non_monorepo.fs = gfs
-    non_monorepo.root_dir = "/"
-
-    assert non_monorepo.subrepo_relpath == ""
-
-    destroy(non_monorepo)
-
-    monorepo_project_a = Repo.init(tmp_dir / "project_a", subdir=True)
-    monorepo_project_a.fs = gfs
-    monorepo_project_a.root_dir = "/project_a"
-
-    assert monorepo_project_a.subrepo_relpath == "project_a"
-
-    monorepo_project_b = Repo.init(tmp_dir / "subdir" / "project_b", subdir=True)
-    monorepo_project_b.fs = gfs
-    monorepo_project_b.root_dir = "/subdir/project_b"
-
-    assert monorepo_project_b.subrepo_relpath == "subdir/project_b"