Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/push.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,22 @@ For large datasets with thousands of files, a single `dvc push` can be slow due

Parallel push distributes the work across multiple nodes, with each worker handling a disjoint, byte-balanced partition of the files. This avoids conflicts, and because no single worker is left holding all the large files, wall-clock time scales with the number of workers rather than with the biggest partition.

## Targets vs remotes

A target names *data* to push -- a tracked path, a `.dvc` file, or a stage
name. The remote is chosen with `-r`, so a bare remote name is read as a
target and nothing matches it:

```bash
$ dt push myremote
Error: No such target: 'myremote' (not a tracked path, a .dvc file, or a stage name)

'myremote' is a configured remote, not a target. To push to it:
dt push -r myremote
```

With no targets at all, `dt push` pushes everything.

## Example

Suppose a project has remotes for cloud storage and HPC as follows:
Expand Down
2 changes: 1 addition & 1 deletion dt/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
"""DVC Tools - Convenient tools for working with DVC in HPC environments."""

__version__ = "0.25.0"
__version__ = "0.25.1"
2 changes: 2 additions & 0 deletions dt/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,8 @@ def collect_hashes_for_targets(

try:
result = utils.collect_tracked_entries(targets=targets, push=False)
except utils.TargetNotFoundError as e:
raise CacheError(str(e))
except utils.DependencyError as e:
raise CacheError(str(e))

Expand Down
6 changes: 4 additions & 2 deletions dt/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -3708,7 +3708,7 @@ def push(ctx, workers, worker, manifest, remote, no_wait, dry, verbose):
try:
# Dry run mode: show what would be pushed
if dry:
targets = [arg for arg in ctx.args if not arg.startswith('-')]
targets = push_mod.extract_targets(list(ctx.args))
manifest = push_mod.build_manifest(
targets=targets if targets else None,
remote=remote,
Expand Down Expand Up @@ -3765,7 +3765,7 @@ def push(ctx, workers, worker, manifest, remote, no_wait, dry, verbose):
# Parallel mode: submit qxub jobs
if workers is not None:
# Extract targets from extra args (non-option args)
targets = [arg for arg in ctx.args if not arg.startswith('-')]
targets = push_mod.extract_targets(list(ctx.args))

# Extract qxub-relevant options if any (could extend later)
qxub_args = None
Expand All @@ -3792,6 +3792,8 @@ def push(ctx, workers, worker, manifest, remote, no_wait, dry, verbose):

# Simple mode: push to all project remotes
# If remote specified, push to just that one
push_mod.check_targets(list(ctx.args))

if remote:
success, output = push_mod.push_to_remote(remote, ctx.args)
status = "✓" if success else "✗"
Expand Down
14 changes: 14 additions & 0 deletions dt/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,20 @@ class RemoteError(DTError):
pass


class TargetNotFoundError(DTError):
"""A target does not name anything DVC tracks.

Raised instead of letting DVC's own collection exception escape, so a
mistyped (or misplaced) target reads as an error message rather than a
traceback. Carries the offending targets so callers can add their own
hint -- ``dt push`` suggests ``-r`` when the target is a remote name.
"""

def __init__(self, message: str, targets=None):
super().__init__(message)
self.targets = list(targets or [])


class StatusError(DTError):
"""Error during status operations."""
pass
Expand Down
84 changes: 84 additions & 0 deletions dt/push.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,88 @@ def push_all(args: List[str]) -> List[Tuple[str, bool, str]]:
return results


#: `dvc push` options that take a separate value. Their value is not a target,
#: so it has to be skipped when picking targets out of the passthrough args.
VALUE_OPTIONS = {'-j', '--jobs', '-r', '--remote', '--rev', '--remote-refresh'}


def extract_targets(args: List[str]) -> List[str]:
"""Pick the targets out of args forwarded to `dvc push`.

Anything option-shaped is dropped, as is the value of an option that takes
one -- otherwise `dt push --dry --jobs 8` reads 8 as a target.
"""
targets = []
skip_next = False
for arg in args:
if skip_next:
skip_next = False
continue
if arg.startswith('-'):
skip_next = arg in VALUE_OPTIONS
continue
targets.append(arg)
return targets


def remote_names() -> Set[str]:
"""Every remote name visible to this repo, at any config scope."""
try:
from . import remote as remote_mod
return {name for name, _url, _default in remote_mod.list_remotes(project_only=False)}
except Exception:
return {name for name, _url in get_project_remotes()}


def explain_bad_target(message: str, targets: Optional[List[str]] = None) -> str:
"""Add a hint to a "no such target" message.

The common way to hit this is forgetting ``-r``: ``dt push myremote``
reads ``myremote`` as data to push, not as the remote to push it to. Say
so when the target really is a remote name; otherwise just say what a
target is.
"""
named = [t for t in (targets or []) if t in remote_names()]

if named:
hint = (
f"\n\n'{named[0]}' is a configured remote, not a target. "
f"To push to it:\n"
f" dt push -r {named[0]}"
)
else:
hint = (
"\n\nA target names data to push -- a tracked path, a .dvc file, "
"or a stage name.\nRun `dt push` with no targets to push everything, "
"or `dt push -r NAME` to choose a remote."
)

return message + hint


def check_targets(args: List[str]) -> None:
"""Reject a passthrough arg that names a remote rather than data.

The simple push path shells out to `dvc push`, which would report the
same argument as a missing stage. Only a target that cannot resolve *and*
names a configured remote is rejected here, so nothing DVC would have
accepted is turned away.

Raises:
PushError: If a target is a remote name.
"""
missing = utils.unresolvable_targets(extract_targets(args))
named = [target for target in missing if target in remote_names()]
if not named:
return

raise PushError(explain_bad_target(
f"No such target: {named[0]!r} "
f"(not a tracked path, a .dvc file, or a stage name)",
named,
))


# =============================================================================
# Parallel push infrastructure
# =============================================================================
Expand Down Expand Up @@ -244,6 +326,8 @@ def build_manifest(
# Use shared helper to collect tracked entries
try:
result = utils.collect_tracked_entries(targets=targets, remote=remote, push=True)
except utils.TargetNotFoundError as e:
raise PushError(explain_bad_target(str(e), e.targets))
except utils.DependencyError as e:
raise PushError(str(e))

Expand Down
117 changes: 102 additions & 15 deletions dt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,41 @@
import csv
import hashlib
import os
import re
import shutil
import socket
import subprocess
from pathlib import Path
from typing import Any, Dict, Iterable, List, Optional, Tuple, Union
from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, Union

from dvc.repo import Repo
from dvc_data.hashfile.hash import file_md5 as _dvc_file_md5
from dvc_data.hashfile.hash_info import HashInfo
from dvc_data.hashfile.meta import Meta
from dvc_data.hashfile.tree import Tree

from .errors import DependencyError, DVCFileError
from .errors import DependencyError, DVCFileError, TargetNotFoundError

# DVC's exceptions for "that target does not name anything I track". Caught
# where we call DVC's collection directly, so a bad target is reported rather
# than raised as a traceback.
try:
from dvc.exceptions import (
NoOutputOrStageError,
OutputNotFoundError,
PathMissingError,
)
from dvc.stage.exceptions import StageFileDoesNotExistError, StageNotFound

_TARGET_COLLECTION_ERRORS = (
NoOutputOrStageError,
OutputNotFoundError,
PathMissingError,
StageFileDoesNotExistError,
StageNotFound,
)
except ImportError: # pragma: no cover - DVC always provides these
_TARGET_COLLECTION_ERRORS = ()


# =============================================================================
Expand Down Expand Up @@ -226,6 +248,67 @@ def _get_dvc_file_metadata(path: str) -> Dict[str, Any]:
return {'size': None, 'nfiles': None}


def _stage_names(dvc_yaml: Path = Path('dvc.yaml')) -> Set[str]:
"""Stage names defined in a dvc.yaml, empty if there isn't one."""
import yaml

if not dvc_yaml.exists():
return set()
try:
with open(dvc_yaml) as f:
data = yaml.safe_load(f) or {}
except (OSError, yaml.YAMLError):
return set()
stages = data.get('stages')
return set(stages) if isinstance(stages, dict) else set()


def unresolvable_targets(targets: Optional[List[str]]) -> List[str]:
"""Targets that cannot name anything DVC tracks.

A target is a path, a .dvc file, or a stage name; anything that is none of
those cannot resolve, whatever the repo state. Kept deliberately
permissive -- an out that is merely not checked out still has its .dvc
file, so it is not reported here.
"""
missing = []
for target in targets or []:
path = Path(target)
if path.exists() or Path(f"{target}.dvc").exists():
continue
# `dvc.yaml:stage` and `subdir/dvc.yaml:stage` name a stage by file.
head, sep, _tail = target.partition(':')
if sep and Path(head).exists():
continue
if target in _stage_names():
continue
# A path inside a tracked directory resolves through the directory's
# .dvc file, even when the directory is not checked out.
if any(Path(f"{parent}.dvc").exists() for parent in path.parents):
continue
missing.append(target)
return missing


def _target_not_found(exc: Exception, targets: Optional[List[str]]) -> TargetNotFoundError:
"""Turn DVC's collection failure into a message that names the target."""
# DVC's messages lead with the offending target in quotes; prefer that,
# since it reflects what DVC actually choked on.
match = re.match(r"'([^']+)'", str(exc))
if match and match.group(1) in (targets or []):
named = [match.group(1)]
else:
named = unresolvable_targets(targets) or list(targets or [])

quoted = ', '.join(repr(t) for t in named)
plural = 's' if len(named) > 1 else ''
return TargetNotFoundError(
f"No such target{plural}: {quoted} "
f"(not a tracked path, a .dvc file, or a stage name)",
targets=named,
)


def collect_tracked_entries(
targets: Optional[List[str]] = None,
remote: Optional[str] = None,
Expand All @@ -251,6 +334,7 @@ def collect_tracked_entries(

Raises:
DependencyError: If DVC internals are not available
TargetNotFoundError: If a target does not name anything DVC tracks
"""
try:
from dvc.repo.fetch import _collect_indexes
Expand All @@ -259,19 +343,22 @@ def collect_tracked_entries(

repo = Repo()

indexes = _collect_indexes(
repo,
targets=targets,
remote=remote,
all_branches=False,
with_deps=False,
all_tags=False,
recursive=False,
all_commits=False,
revs=None,
workspace=True,
push=push,
)
try:
indexes = _collect_indexes(
repo,
targets=targets,
remote=remote,
all_branches=False,
with_deps=False,
all_tags=False,
recursive=False,
all_commits=False,
revs=None,
workspace=True,
push=push,
)
except _TARGET_COLLECTION_ERRORS as e:
raise _target_not_found(e, targets) from e

if not indexes:
return {
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"

[project]
name = "dvc-tools"
version = "0.25.0"
version = "0.25.1"
description = "Convenient tools for working with DVC in HPC environments"
readme = "README.md"
requires-python = ">=3.8"
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class TestExceptionHierarchy:
errors.PushError,
errors.RemoteError,
errors.SummaryError,
errors.TargetNotFoundError,
errors.TmpError,
errors.WorktreeError,
errors.AddError,
Expand Down Expand Up @@ -80,6 +81,7 @@ def test_inherits_from_dterror(self, exception_class):
errors.PushError,
errors.RemoteError,
errors.SummaryError,
errors.TargetNotFoundError,
errors.TmpError,
errors.WorktreeError,
errors.AddError,
Expand Down Expand Up @@ -111,6 +113,7 @@ def test_can_be_raised_and_caught(self, exception_class):
errors.PushError,
errors.RemoteError,
errors.SummaryError,
errors.TargetNotFoundError,
errors.TmpError,
errors.WorktreeError,
errors.AddError,
Expand Down Expand Up @@ -220,3 +223,16 @@ def test_exception_chain(self):


# Run with: pytest tests/test_errors.py -v


class TestTargetNotFoundError:
"""TargetNotFoundError carries the offending targets."""

def test_stores_targets(self):
err = errors.TargetNotFoundError("No such target: 'x'", targets=["x"])
assert err.targets == ["x"]
assert str(err) == "No such target: 'x'"

def test_targets_default_to_empty(self):
err = errors.TargetNotFoundError("No such target")
assert err.targets == []
Loading
Loading