From 76b8847e8ba033bf216e9af21756a774ffd8d6f5 Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Thu, 22 May 2025 10:52:48 +0200 Subject: [PATCH 01/15] =?UTF-8?q?=E2=9C=A8=20Add=20outs=5Fno=5Fpush=20stag?= =?UTF-8?q?e=20option?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dvc/stage/utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dvc/stage/utils.py b/dvc/stage/utils.py index 9e335cf3e2..9fe25dd2cc 100644 --- a/dvc/stage/utils.py +++ b/dvc/stage/utils.py @@ -62,6 +62,7 @@ def fill_stage_outputs(stage, **kwargs): "plots_persist_no_cache", "outs_no_cache", "outs", + "outs_no_push", ] stage.outs = [] @@ -74,6 +75,7 @@ def fill_stage_outputs(stage, **kwargs): persist="persist" in key, metric="metrics" in key, plot="plots" in key, + push="no_push" not in key, ) From c01004ed79aa1177be910be611abe6841eae53f4 Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Thu, 22 May 2025 13:17:27 +0200 Subject: [PATCH 02/15] =?UTF-8?q?=F0=9F=A7=AA=20Test=20ignore=20not=5Fin?= =?UTF-8?q?=5Fremote=20when=20push=20false?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/func/test_data_status.py | 38 +++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/tests/func/test_data_status.py b/tests/func/test_data_status.py index b7d0c97c41..ce86eed655 100644 --- a/tests/func/test_data_status.py +++ b/tests/func/test_data_status.py @@ -5,7 +5,7 @@ from dvc.repo import Repo from dvc.repo.data import _transform_git_paths_to_dvc, posixpath_to_os_path -from dvc.testing.tmp_dir import make_subrepo +from dvc.testing.tmp_dir import TmpDir, make_subrepo from dvc.utils.fs import remove EMPTY_STATUS = { @@ -423,6 +423,42 @@ def test_missing_remote_cache(M, tmp_dir, dvc, scm, local_remote): } +@pytest.fixture +def dvc_pipeline_with_push_false( + tmp_dir: TmpDir, dvc: Repo, scm, mocker, local_remote +) -> None: + """DVC pipeline with with two outs, one with push=False.""" + tmp_dir.gen("fixed", "fixed") + dvc.stage.add( + name="create-foo", cmd="echo foo > foo", deps=["fixed"], outs_no_push=["foo"] + ) + dvc.stage.add(name="create-bar", cmd="echo bar > bar", deps=["fixed"], outs=["bar"]) + dvc.reproduce() + + assert set( + dvc.data_status(remote_refresh=True, not_in_remote=True)["not_in_remote"] + ) == {"foo", "bar"} + + +def test_missing_remote_push_false(dvc_pipeline_with_push_false: None, dvc: TmpDir): + dvc.push() + assert set( + dvc.data_status(remote_refresh=True, not_in_remote=True)["not_in_remote"] + ) == {"foo"} + + +def test_missing_remote_push_false_respects_no_push_flag( + dvc_pipeline_with_push_false: None, dvc: TmpDir +): + dvc.push() + assert ( + dvc.data_status( + remote_refresh=True, not_in_remote=True, not_in_remote_no_push=True + )["not_in_remote"] + == [] + ) + + def test_root_from_dir_to_file(M, tmp_dir, dvc, scm): tmp_dir.dvc_gen({"data": {"foo": "foo", "bar": "bar"}}) remove("data") From afa793a7fbc81bf727268713a1986bcb16f6adc1 Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Thu, 22 May 2025 13:21:14 +0200 Subject: [PATCH 03/15] =?UTF-8?q?=E2=9C=A8=20Add=20respect=5Fno=5Fpush=20f?= =?UTF-8?q?lag=20to=20status?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dvc/repo/data.py | 36 ++++++++++++++++++++++++++++++++-- tests/func/test_data_status.py | 13 ++++++------ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/dvc/repo/data.py b/dvc/repo/data.py index bc727c88c7..0c2a5874aa 100644 --- a/dvc/repo/data.py +++ b/dvc/repo/data.py @@ -4,8 +4,11 @@ from typing import TYPE_CHECKING, Any, TypedDict, Union from dvc.fs.callbacks import DEFAULT_CALLBACK +from dvc.log import logger from dvc.ui import ui +logger = logger.getChild(__name__) + if TYPE_CHECKING: from dvc.fs.callbacks import Callback from dvc.repo import Repo @@ -217,7 +220,29 @@ def _transform_git_paths_to_dvc(repo: "Repo", files: Iterable[str]) -> list[str] return [repo.fs.relpath(file, start) for file in files] -def status(repo: "Repo", untracked_files: str = "no", **kwargs: Any) -> Status: +def _filter_out_push_false_outs(repo: "Repo", not_in_remote: list[str]) -> list[str]: + """Filter out paths that are not pushable.""" + filtered_not_in_remote = [] + + for path in not_in_remote: + (out,) = repo.find_outs_by_path(path) + + if out.can_push: + filtered_not_in_remote.append(path) + else: + logger.trace( + f"Eliminating {path} from not_in_remote, because it is not pushable" + ) + + return filtered_not_in_remote + + +def status( + repo: "Repo", + untracked_files: str = "no", + respect_no_push: bool = False, + **kwargs: Any, +) -> Status: from dvc.scm import NoSCMError, SCMError head = kwargs.pop("head", "HEAD") @@ -234,10 +259,17 @@ def status(repo: "Repo", untracked_files: str = "no", **kwargs: Any) -> Status: git_info = _git_info(repo.scm, untracked_files=untracked_files) untracked = git_info.get("untracked", []) untracked = _transform_git_paths_to_dvc(repo, untracked) + + not_in_remote = uncommitted_diff.pop("not_in_remote", []) + + if respect_no_push: + logger.debug("Filtering out paths that are not pushable") + not_in_remote = _filter_out_push_false_outs(repo, not_in_remote) + # order matters here return Status( not_in_cache=uncommitted_diff.pop("not_in_cache", []), - not_in_remote=uncommitted_diff.pop("not_in_remote", []), + not_in_remote=not_in_remote, committed=committed_diff, uncommitted=uncommitted_diff, untracked=untracked, diff --git a/tests/func/test_data_status.py b/tests/func/test_data_status.py index ce86eed655..fdb136e8da 100644 --- a/tests/func/test_data_status.py +++ b/tests/func/test_data_status.py @@ -438,23 +438,22 @@ def dvc_pipeline_with_push_false( assert set( dvc.data_status(remote_refresh=True, not_in_remote=True)["not_in_remote"] ) == {"foo", "bar"} + dvc.push() -def test_missing_remote_push_false(dvc_pipeline_with_push_false: None, dvc: TmpDir): - dvc.push() +def test_missing_remote_push_false(dvc_pipeline_with_push_false: None, dvc: Repo): assert set( dvc.data_status(remote_refresh=True, not_in_remote=True)["not_in_remote"] ) == {"foo"} def test_missing_remote_push_false_respects_no_push_flag( - dvc_pipeline_with_push_false: None, dvc: TmpDir + dvc_pipeline_with_push_false: None, dvc: Repo ): - dvc.push() assert ( - dvc.data_status( - remote_refresh=True, not_in_remote=True, not_in_remote_no_push=True - )["not_in_remote"] + dvc.data_status(remote_refresh=True, not_in_remote=True, respect_no_push=True)[ + "not_in_remote" + ] == [] ) From 125db32dfb517d144d2a61c0191d4473f077fa3f Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Thu, 22 May 2025 13:58:09 +0200 Subject: [PATCH 04/15] =?UTF-8?q?=E2=9C=A8=20Add=20--respect-no-flag=20to?= =?UTF-8?q?=20dvc=20data=20status=20cli=20command?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dvc/commands/data.py | 7 +++++++ tests/unit/command/test_data_status.py | 1 + 2 files changed, 8 insertions(+) diff --git a/dvc/commands/data.py b/dvc/commands/data.py index a9ea6e038e..d86aa6bb08 100644 --- a/dvc/commands/data.py +++ b/dvc/commands/data.py @@ -112,6 +112,7 @@ def run(self) -> int: untracked_files=self.args.untracked_files, not_in_remote=self.args.not_in_remote, remote_refresh=self.args.remote_refresh, + respect_no_push=self.args.respect_no_push, ) if not self.args.unchanged: @@ -179,6 +180,12 @@ def add_parser(subparsers, parent_parser): default=False, help="Show files not in remote.", ) + data_status_parser.add_argument( + "--respect-no-push", + action="store_true", + default=False, + help="Respect the `push: false` flag in the DVC stage's outs.", + ) data_status_parser.add_argument( "--no-remote-refresh", dest="remote_refresh", diff --git a/tests/unit/command/test_data_status.py b/tests/unit/command/test_data_status.py index f37d270a60..904db3db94 100644 --- a/tests/unit/command/test_data_status.py +++ b/tests/unit/command/test_data_status.py @@ -52,6 +52,7 @@ def test_cli(dvc, mocker, mocked_status): status.assert_called_once_with( untracked_files="all", not_in_remote=False, + respect_no_push=False, remote_refresh=True, granular=True, ) From 8328efcc89b19b7aefbacd0f47aea2273cfcb49c Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Tue, 10 Jun 2025 10:58:54 +0200 Subject: [PATCH 05/15] Set respect-no-push default True --- dvc/commands/data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/commands/data.py b/dvc/commands/data.py index d86aa6bb08..8402f77aad 100644 --- a/dvc/commands/data.py +++ b/dvc/commands/data.py @@ -183,7 +183,7 @@ def add_parser(subparsers, parent_parser): data_status_parser.add_argument( "--respect-no-push", action="store_true", - default=False, + default=True, help="Respect the `push: false` flag in the DVC stage's outs.", ) data_status_parser.add_argument( From 75f1781fd90b98e4c3ea5dc140f5fe8c32840db5 Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Tue, 10 Jun 2025 10:59:36 +0200 Subject: [PATCH 06/15] =?UTF-8?q?Revert=20"=E2=9C=A8=20Add=20outs=5Fno=5Fp?= =?UTF-8?q?ush=20stage=20option"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 2da2f6654da7fa6534d0d12bf13a1d00d24a803f. --- dvc/stage/utils.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/dvc/stage/utils.py b/dvc/stage/utils.py index 9fe25dd2cc..9e335cf3e2 100644 --- a/dvc/stage/utils.py +++ b/dvc/stage/utils.py @@ -62,7 +62,6 @@ def fill_stage_outputs(stage, **kwargs): "plots_persist_no_cache", "outs_no_cache", "outs", - "outs_no_push", ] stage.outs = [] @@ -75,7 +74,6 @@ def fill_stage_outputs(stage, **kwargs): persist="persist" in key, metric="metrics" in key, plot="plots" in key, - push="no_push" not in key, ) From b75efd262099b1b18062047fd2b55a2f59169faa Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Thu, 19 Jun 2025 14:35:42 +0200 Subject: [PATCH 07/15] =?UTF-8?q?Revert=20"=E2=9C=A8=20Add=20--respect-no-?= =?UTF-8?q?flag=20to=20dvc=20data=20status=20cli=20command"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit b5a6a5839206c6120e8c27c90bab44d1217945f0. --- dvc/commands/data.py | 7 ------- tests/unit/command/test_data_status.py | 1 - 2 files changed, 8 deletions(-) diff --git a/dvc/commands/data.py b/dvc/commands/data.py index 8402f77aad..a9ea6e038e 100644 --- a/dvc/commands/data.py +++ b/dvc/commands/data.py @@ -112,7 +112,6 @@ def run(self) -> int: untracked_files=self.args.untracked_files, not_in_remote=self.args.not_in_remote, remote_refresh=self.args.remote_refresh, - respect_no_push=self.args.respect_no_push, ) if not self.args.unchanged: @@ -180,12 +179,6 @@ def add_parser(subparsers, parent_parser): default=False, help="Show files not in remote.", ) - data_status_parser.add_argument( - "--respect-no-push", - action="store_true", - default=True, - help="Respect the `push: false` flag in the DVC stage's outs.", - ) data_status_parser.add_argument( "--no-remote-refresh", dest="remote_refresh", diff --git a/tests/unit/command/test_data_status.py b/tests/unit/command/test_data_status.py index 904db3db94..f37d270a60 100644 --- a/tests/unit/command/test_data_status.py +++ b/tests/unit/command/test_data_status.py @@ -52,7 +52,6 @@ def test_cli(dvc, mocker, mocked_status): status.assert_called_once_with( untracked_files="all", not_in_remote=False, - respect_no_push=False, remote_refresh=True, granular=True, ) From 2858738b39e674a1d712f744bf2ab69e6051ee15 Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Thu, 19 Jun 2025 14:47:28 +0200 Subject: [PATCH 08/15] Use worktree_view as index for not in remote --- dvc/repo/data.py | 60 ++++++++++++++++++++-------------- tests/func/test_data_status.py | 34 ++++--------------- 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/dvc/repo/data.py b/dvc/repo/data.py index 0c2a5874aa..faccc727a5 100644 --- a/dvc/repo/data.py +++ b/dvc/repo/data.py @@ -1,19 +1,17 @@ import os import posixpath -from collections.abc import Iterable +from collections.abc import Iterable, Iterator from typing import TYPE_CHECKING, Any, TypedDict, Union from dvc.fs.callbacks import DEFAULT_CALLBACK -from dvc.log import logger +from dvc.repo.worktree import worktree_view from dvc.ui import ui -logger = logger.getChild(__name__) - if TYPE_CHECKING: from dvc.fs.callbacks import Callback from dvc.repo import Repo from dvc.scm import Git, NoSCM - from dvc_data.index import DataIndex + from dvc_data.index import DataIndex, DataIndexView from dvc_data.index.diff import Change @@ -220,27 +218,32 @@ def _transform_git_paths_to_dvc(repo: "Repo", files: Iterable[str]) -> list[str] return [repo.fs.relpath(file, start) for file in files] -def _filter_out_push_false_outs(repo: "Repo", not_in_remote: list[str]) -> list[str]: - """Filter out paths that are not pushable.""" - filtered_not_in_remote = [] - - for path in not_in_remote: - (out,) = repo.find_outs_by_path(path) +def _get_not_in_remote( + data_index: Union["DataIndex", "DataIndexView"], + remote_refresh: bool, + shallow: bool, +) -> Iterator[str]: + """Get entries that are not in remote storage.""" + from dvc_data.index import StorageKeyError - if out.can_push: - filtered_not_in_remote.append(path) - else: - logger.trace( - f"Eliminating {path} from not_in_remote, because it is not pushable" - ) + for key, entry in data_index.iteritems(shallow=shallow): + if not (entry and entry.hash_info): + continue - return filtered_not_in_remote + k = (*key, "") if entry.meta and entry.meta.isdir else key + try: + if not data_index.storage_map.remote_exists( + entry, remote_refresh=remote_refresh + ): + yield os.path.sep.join(k) + except StorageKeyError: + pass def status( repo: "Repo", untracked_files: str = "no", - respect_no_push: bool = False, + not_in_remote: bool = False, **kwargs: Any, ) -> Status: from dvc.scm import NoSCMError, SCMError @@ -260,16 +263,23 @@ def status( untracked = git_info.get("untracked", []) untracked = _transform_git_paths_to_dvc(repo, untracked) - not_in_remote = uncommitted_diff.pop("not_in_remote", []) - - if respect_no_push: - logger.debug("Filtering out paths that are not pushable") - not_in_remote = _filter_out_push_false_outs(repo, not_in_remote) + if not_in_remote: + # View into the index, with only pushable entries + view = worktree_view(repo.index, push=True).data["repo"] + entries_not_in_remote = list( + _get_not_in_remote( + view, + remote_refresh=kwargs["not_in_remote"], + shallow=not kwargs["granular"], + ) + ) + else: + entries_not_in_remote = [] # order matters here return Status( not_in_cache=uncommitted_diff.pop("not_in_cache", []), - not_in_remote=not_in_remote, + not_in_remote=entries_not_in_remote, committed=committed_diff, uncommitted=uncommitted_diff, untracked=untracked, diff --git a/tests/func/test_data_status.py b/tests/func/test_data_status.py index fdb136e8da..e7b18766d7 100644 --- a/tests/func/test_data_status.py +++ b/tests/func/test_data_status.py @@ -423,39 +423,19 @@ def test_missing_remote_cache(M, tmp_dir, dvc, scm, local_remote): } -@pytest.fixture -def dvc_pipeline_with_push_false( +def test_not_in_remote_respects_not_pushable( tmp_dir: TmpDir, dvc: Repo, scm, mocker, local_remote -) -> None: - """DVC pipeline with with two outs, one with push=False.""" - tmp_dir.gen("fixed", "fixed") - dvc.stage.add( - name="create-foo", cmd="echo foo > foo", deps=["fixed"], outs_no_push=["foo"] - ) - dvc.stage.add(name="create-bar", cmd="echo bar > bar", deps=["fixed"], outs=["bar"]) - dvc.reproduce() +): + stage = dvc.stage.create(outs=["foo"], single_stage=True) + stage.outs[0].can_push = False + stage.dump() - assert set( - dvc.data_status(remote_refresh=True, not_in_remote=True)["not_in_remote"] - ) == {"foo", "bar"} + tmp_dir.dvc_gen({"dir": {"foobar": "foobar"}}) dvc.push() - -def test_missing_remote_push_false(dvc_pipeline_with_push_false: None, dvc: Repo): assert set( dvc.data_status(remote_refresh=True, not_in_remote=True)["not_in_remote"] - ) == {"foo"} - - -def test_missing_remote_push_false_respects_no_push_flag( - dvc_pipeline_with_push_false: None, dvc: Repo -): - assert ( - dvc.data_status(remote_refresh=True, not_in_remote=True, respect_no_push=True)[ - "not_in_remote" - ] - == [] - ) + ) == {"dir/", "dir/foobar"} def test_root_from_dir_to_file(M, tmp_dir, dvc, scm): From 3d3903c7c15f8a942b4cf03f0c11c7bcd5a5fc55 Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Thu, 19 Jun 2025 15:36:43 +0200 Subject: [PATCH 09/15] Fix missing kwargs --- dvc/repo/data.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/dvc/repo/data.py b/dvc/repo/data.py index faccc727a5..6e56e31e1f 100644 --- a/dvc/repo/data.py +++ b/dvc/repo/data.py @@ -232,9 +232,7 @@ def _get_not_in_remote( k = (*key, "") if entry.meta and entry.meta.isdir else key try: - if not data_index.storage_map.remote_exists( - entry, remote_refresh=remote_refresh - ): + if not data_index.storage_map.remote_exists(entry, refresh=remote_refresh): yield os.path.sep.join(k) except StorageKeyError: pass @@ -269,8 +267,8 @@ def status( entries_not_in_remote = list( _get_not_in_remote( view, - remote_refresh=kwargs["not_in_remote"], - shallow=not kwargs["granular"], + remote_refresh=not_in_remote, + shallow=not kwargs.get("granular", False), ) ) else: From 100983e277338c482855ccb304d6f493e0bab4d1 Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Thu, 19 Jun 2025 15:37:55 +0200 Subject: [PATCH 10/15] Make test more robust --- tests/func/test_data_status.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/func/test_data_status.py b/tests/func/test_data_status.py index e7b18766d7..c9102e2635 100644 --- a/tests/func/test_data_status.py +++ b/tests/func/test_data_status.py @@ -1,5 +1,6 @@ from os import fspath from os.path import join +from typing import TYPE_CHECKING import pytest @@ -8,6 +9,9 @@ from dvc.testing.tmp_dir import TmpDir, make_subrepo from dvc.utils.fs import remove +if TYPE_CHECKING: + from dvc.stage import Stage + EMPTY_STATUS = { "committed": {}, "uncommitted": {}, @@ -426,16 +430,24 @@ def test_missing_remote_cache(M, tmp_dir, dvc, scm, local_remote): def test_not_in_remote_respects_not_pushable( tmp_dir: TmpDir, dvc: Repo, scm, mocker, local_remote ): - stage = dvc.stage.create(outs=["foo"], single_stage=True) - stage.outs[0].can_push = False - stage.dump() + stages: list[Stage] = tmp_dir.dvc_gen({"foo": "foo", "dir": {"foobar": "foobar"}}) + # Make foo not pushable + stages[0].outs[0].can_push = False + stages[0].dump() + + def assert_not_in_remote_is(granular: bool, expected: set[str]): + not_in_remote = dvc.data_status( + granular=granular, remote_refresh=True, not_in_remote=True + )["not_in_remote"] + assert set(not_in_remote) == expected + + assert_not_in_remote_is(granular=True, expected={"dir/", "dir/foobar"}) + assert_not_in_remote_is(granular=False, expected={"dir/"}) - tmp_dir.dvc_gen({"dir": {"foobar": "foobar"}}) dvc.push() - assert set( - dvc.data_status(remote_refresh=True, not_in_remote=True)["not_in_remote"] - ) == {"dir/", "dir/foobar"} + assert_not_in_remote_is(granular=True, expected=set()) + assert_not_in_remote_is(granular=False, expected=set()) def test_root_from_dir_to_file(M, tmp_dir, dvc, scm): From 308b84bbf2d3dca7cb2f10211200416f4d1a88ba Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Thu, 19 Jun 2025 15:57:52 +0200 Subject: [PATCH 11/15] Fix windows style dir separator in test --- tests/func/test_data_status.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/func/test_data_status.py b/tests/func/test_data_status.py index c9102e2635..2c0f77651e 100644 --- a/tests/func/test_data_status.py +++ b/tests/func/test_data_status.py @@ -435,19 +435,19 @@ def test_not_in_remote_respects_not_pushable( stages[0].outs[0].can_push = False stages[0].dump() - def assert_not_in_remote_is(granular: bool, expected: set[str]): + def assert_not_in_remote_is(granular: bool, expected: list[str]): not_in_remote = dvc.data_status( granular=granular, remote_refresh=True, not_in_remote=True )["not_in_remote"] - assert set(not_in_remote) == expected + assert set(not_in_remote) == set(map(posixpath_to_os_path, expected)) - assert_not_in_remote_is(granular=True, expected={"dir/", "dir/foobar"}) - assert_not_in_remote_is(granular=False, expected={"dir/"}) + assert_not_in_remote_is(granular=True, expected=["dir/", "dir/foobar"]) + assert_not_in_remote_is(granular=False, expected=["dir/"]) dvc.push() - assert_not_in_remote_is(granular=True, expected=set()) - assert_not_in_remote_is(granular=False, expected=set()) + assert_not_in_remote_is(granular=True, expected=[]) + assert_not_in_remote_is(granular=False, expected=[]) def test_root_from_dir_to_file(M, tmp_dir, dvc, scm): From 3a22912aea32e611645bf4cf1bdd1823ccb48b6d Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Fri, 20 Jun 2025 08:25:43 +0200 Subject: [PATCH 12/15] Remove old not_in_remote remote_refresh from _diff --- dvc/repo/data.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/dvc/repo/data.py b/dvc/repo/data.py index 6e56e31e1f..f837c64c60 100644 --- a/dvc/repo/data.py +++ b/dvc/repo/data.py @@ -52,11 +52,8 @@ def _diff( *, granular: bool = False, not_in_cache: bool = False, - not_in_remote: bool = False, - remote_refresh: bool = False, callback: "Callback" = DEFAULT_CALLBACK, ) -> dict[str, list[str]]: - from dvc_data.index import StorageError from dvc_data.index.diff import UNCHANGED, UNKNOWN, diff ret: dict[str, list[str]] = {} @@ -98,19 +95,6 @@ def _add_change(typ, change): # NOTE: emulating previous behaviour _add_change("not_in_cache", change) - try: - if ( - not_in_remote - and change.old - and change.old.hash_info - and not old.storage_map.remote_exists( - change.old, refresh=remote_refresh - ) - ): - _add_change("not_in_remote", change) - except StorageError: - pass - _add_change(change.typ, change) return ret From 2ccc9d537df433fc66940a847c09485550605acc Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Fri, 20 Jun 2025 08:45:36 +0200 Subject: [PATCH 13/15] Refactor status Convert kwargs to explicit arguments and extract not_in_remote handling --- dvc/repo/data.py | 54 ++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/dvc/repo/data.py b/dvc/repo/data.py index f837c64c60..6387587f7e 100644 --- a/dvc/repo/data.py +++ b/dvc/repo/data.py @@ -1,6 +1,6 @@ import os import posixpath -from collections.abc import Iterable, Iterator +from collections.abc import Iterable from typing import TYPE_CHECKING, Any, TypedDict, Union from dvc.fs.callbacks import DEFAULT_CALLBACK @@ -11,7 +11,7 @@ from dvc.fs.callbacks import Callback from dvc.repo import Repo from dvc.scm import Git, NoSCM - from dvc_data.index import DataIndex, DataIndexView + from dvc_data.index import DataIndex from dvc_data.index.diff import Change @@ -202,40 +202,47 @@ def _transform_git_paths_to_dvc(repo: "Repo", files: Iterable[str]) -> list[str] return [repo.fs.relpath(file, start) for file in files] -def _get_not_in_remote( - data_index: Union["DataIndex", "DataIndexView"], - remote_refresh: bool, - shallow: bool, -) -> Iterator[str]: +def _get_entries_not_in_remote( + repo: "Repo", + granular: bool = False, + remote_refresh: bool = False, +) -> list[str]: """Get entries that are not in remote storage.""" from dvc_data.index import StorageKeyError - for key, entry in data_index.iteritems(shallow=shallow): + # View into the index, with only pushable entries + view = worktree_view(repo.index, push=True).data["repo"] + + missing_entries = [] + for key, entry in view.iteritems(shallow=not granular): if not (entry and entry.hash_info): continue k = (*key, "") if entry.meta and entry.meta.isdir else key try: - if not data_index.storage_map.remote_exists(entry, refresh=remote_refresh): - yield os.path.sep.join(k) + if not view.storage_map.remote_exists(entry, refresh=remote_refresh): + missing_entries.append(os.path.sep.join(k)) except StorageKeyError: pass + return missing_entries + def status( repo: "Repo", untracked_files: str = "no", not_in_remote: bool = False, - **kwargs: Any, + remote_refresh: bool = False, + granular: bool = False, + head: str = "HEAD", ) -> Status: from dvc.scm import NoSCMError, SCMError - head = kwargs.pop("head", "HEAD") - uncommitted_diff = _diff_index_to_wtree(repo, **kwargs) + uncommitted_diff = _diff_index_to_wtree(repo, granular=granular) unchanged = set(uncommitted_diff.pop("unchanged", [])) try: - committed_diff = _diff_head_to_index(repo, head=head, **kwargs) + committed_diff = _diff_head_to_index(repo, head=head, granular=granular) except (SCMError, NoSCMError): committed_diff = {} else: @@ -245,18 +252,15 @@ def status( untracked = git_info.get("untracked", []) untracked = _transform_git_paths_to_dvc(repo, untracked) - if not_in_remote: - # View into the index, with only pushable entries - view = worktree_view(repo.index, push=True).data["repo"] - entries_not_in_remote = list( - _get_not_in_remote( - view, - remote_refresh=not_in_remote, - shallow=not kwargs.get("granular", False), - ) + entries_not_in_remote = ( + _get_entries_not_in_remote( + repo, + granular=granular, + remote_refresh=remote_refresh, ) - else: - entries_not_in_remote = [] + if not_in_remote + else [] + ) # order matters here return Status( From bc093c50434e70bd87756b271bfe5257aaaa2339 Mon Sep 17 00:00:00 2001 From: "Thorvald M. Ballestad" Date: Fri, 20 Jun 2025 10:12:52 +0200 Subject: [PATCH 14/15] Refactor test to use convention --- tests/func/test_data_status.py | 42 ++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/tests/func/test_data_status.py b/tests/func/test_data_status.py index 2c0f77651e..1779ecd27d 100644 --- a/tests/func/test_data_status.py +++ b/tests/func/test_data_status.py @@ -10,6 +10,8 @@ from dvc.utils.fs import remove if TYPE_CHECKING: + from pytest_test_utils import matchers + from dvc.stage import Stage EMPTY_STATUS = { @@ -428,26 +430,48 @@ def test_missing_remote_cache(M, tmp_dir, dvc, scm, local_remote): def test_not_in_remote_respects_not_pushable( - tmp_dir: TmpDir, dvc: Repo, scm, mocker, local_remote + M: type["matchers.Matcher"], tmp_dir: TmpDir, dvc: Repo, scm, mocker, local_remote ): stages: list[Stage] = tmp_dir.dvc_gen({"foo": "foo", "dir": {"foobar": "foobar"}}) # Make foo not pushable stages[0].outs[0].can_push = False stages[0].dump() - def assert_not_in_remote_is(granular: bool, expected: list[str]): - not_in_remote = dvc.data_status( + def assert_not_in_remote_is( + granular: bool, not_in_remote: list[str], committed: list[str] + ): + assert dvc.data_status( granular=granular, remote_refresh=True, not_in_remote=True - )["not_in_remote"] - assert set(not_in_remote) == set(map(posixpath_to_os_path, expected)) + ) == { + **EMPTY_STATUS, + "git": M.dict(), + "not_in_remote": M.unordered(*not_in_remote), + "committed": {"added": M.unordered(*committed)}, + } - assert_not_in_remote_is(granular=True, expected=["dir/", "dir/foobar"]) - assert_not_in_remote_is(granular=False, expected=["dir/"]) + foo = "foo" + dir_ = join("dir", "") + foobar = join("dir", "foobar") + + assert_not_in_remote_is( + granular=True, + not_in_remote=[dir_, foobar], + committed=[foo, dir_, foobar], + ) + assert_not_in_remote_is(granular=False, not_in_remote=[dir_], committed=[foo, dir_]) dvc.push() - assert_not_in_remote_is(granular=True, expected=[]) - assert_not_in_remote_is(granular=False, expected=[]) + assert_not_in_remote_is( + granular=True, + not_in_remote=[], + committed=[foo, dir_, foobar], + ) + assert_not_in_remote_is( + granular=False, + not_in_remote=[], + committed=[foo, dir_], + ) def test_root_from_dir_to_file(M, tmp_dir, dvc, scm): From b6b18ef4d7084ac49acf7644cb2b45b8d73a192c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saugat=20Pachhai=20=28=E0=A4=B8=E0=A5=8C=E0=A4=97=E0=A4=BE?= =?UTF-8?q?=E0=A4=A4=29?= Date: Thu, 26 Jun 2025 14:40:05 +0545 Subject: [PATCH 15/15] move _get_entries_not_in_remote check to after _diff_index_to_wtree --- dvc/repo/data.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/dvc/repo/data.py b/dvc/repo/data.py index 6387587f7e..b31d43da72 100644 --- a/dvc/repo/data.py +++ b/dvc/repo/data.py @@ -241,6 +241,16 @@ def status( uncommitted_diff = _diff_index_to_wtree(repo, granular=granular) unchanged = set(uncommitted_diff.pop("unchanged", [])) + entries_not_in_remote = ( + _get_entries_not_in_remote( + repo, + granular=granular, + remote_refresh=remote_refresh, + ) + if not_in_remote + else [] + ) + try: committed_diff = _diff_head_to_index(repo, head=head, granular=granular) except (SCMError, NoSCMError): @@ -252,16 +262,6 @@ def status( untracked = git_info.get("untracked", []) untracked = _transform_git_paths_to_dvc(repo, untracked) - entries_not_in_remote = ( - _get_entries_not_in_remote( - repo, - granular=granular, - remote_refresh=remote_refresh, - ) - if not_in_remote - else [] - ) - # order matters here return Status( not_in_cache=uncommitted_diff.pop("not_in_cache", []),