Stone_Red/dvc
1
0
Fork 0
mirror of https://github.com/Stone-Red-Code/dvc.git synced 2026-11-09 07:18:27 +01:00
Code Issues Packages Projects Releases Wiki Activity

dvc.objects: use stub ignore in dvc.objects/dvc.data

Browse Source 
This commit:
* defines a `Ignore` protocol in `dvc.objects`, that expects a `find` method.
  This will be used in dvc-data and dvc-objects instead of depending on dvc.ignore.
  dvcignore is expected to respect this protocol, which it does.
* renames `dvcignore` kwarg to `ignore` in `dvc-data` and `dvc-objects`.
* teaches dvc.cli that `IgnoreInCollectedError` is a known exception
   and is logged correctly and adds a test for the behaviour.

I am not happy that we need to hardcode `.dvcignore` in dvc-data
but I don't see any easy way, since we always try to avoid
staging if the tree contains .dvcignore file, even if .dvcignore
is not passed.
This commit is contained in:
Saugat Pachhai (सौगात)
2022-05-18 12:22:28 +03:00
committed by Ruslan Kuprieiev
parent 54917e3255
commit 32cd6c2c03
13 changed files with 125 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
dvc/cli/__init__.py
+37 -20
View File
@@ -6,6 +6,7 @@ import sys
# Workaround for CPython bug. See [1] and [2] for more info.
# [1] https://github.com/aws/aws-cli/blob/1.16.277/awscli/clidriver.py#L55
# [2] https://bugs.python.org/issue29288
from typing import Optional
"".encode("idna")
@@ -36,11 +37,37 @@ def parse_args(argv=None):
return args
def _log_exceptions(exc: Exception):
def _log_unknown_exceptions() -> None:
from dvc.info import get_dvc_info
from dvc.logger import FOOTER
logger.exception("unexpected error")
if logger.isEnabledFor(logging.DEBUG):
logger.debug("Version info for developers:\n%s", get_dvc_info())
logger.info(FOOTER)
return None
def _log_exceptions(exc: Exception) -> Optional[int]:
"""Try to log some known exceptions, that are not DVCExceptions."""
from dvc.fs import AuthError, ConfigError, RemoteMissingDepsError
from dvc.utils import error_link, format_link
if isinstance(exc, OSError):
import errno
if exc.errno == errno.EMFILE:
logger.exception(
"too many open files, please visit "
"{} to see how to handle this "
"problem".format(error_link("many-files")),
extra={"tb_only": True},
)
else:
_log_unknown_exceptions()
return None
from dvc.fs import AuthError, ConfigError, RemoteMissingDepsError
if isinstance(exc, RemoteMissingDepsError):
from dvc.utils.pkg import PKG
@@ -69,7 +96,7 @@ def _log_exceptions(exc: Exception):
f"dependencies: {exc.missing_deps}. {hint}",
extra={"tb_only": True},
)
return
return None
if isinstance(exc, (AuthError, ConfigError)):
link = format_link("https://man.dvc.org/remote/modify")
@@ -92,26 +119,16 @@ def _log_exceptions(exc: Exception):
f"\nSee {error_link('pickle')} for more information.",
extra={"tb_only": True},
)
return
return None
if isinstance(exc, OSError):
import errno
from dvc.data.stage import IgnoreInCollectedDirError
if exc.errno == errno.EMFILE:
logger.exception(
"too many open files, please visit "
"{} to see how to handle this "
"problem".format(error_link("many-files")),
extra={"tb_only": True},
)
return
if isinstance(exc, IgnoreInCollectedDirError):
logger.exception("")
return None
from dvc.info import get_dvc_info
from dvc.logger import FOOTER
logger.exception("unexpected error")
logger.debug("Version info for developers:\n%s", get_dvc_info())
logger.info(FOOTER)
_log_unknown_exceptions()
return None
def main(argv=None): # noqa: C901
dvc/data/checkout.py
+8 -6
View File
@@ -1,10 +1,9 @@
import logging
from itertools import chain
from typing import Optional
from typing import TYPE_CHECKING, Optional
from dvc import prompt
from dvc.exceptions import CacheLinkError, CheckoutError, ConfirmRemoveError
from dvc.ignore import DvcIgnoreFilter
from dvc.objects.fs._callback import FsspecCallback
from dvc.objects.fs.generic import test_links, transfer
@@ -13,6 +12,9 @@ from .diff import diff as odiff
from .slow_link_detection import slow_link_guard # type: ignore[attr-defined]
from .stage import stage
if TYPE_CHECKING:
from dvc.objects._ignore import Ignore
logger = logging.getLogger(__name__)
@@ -101,7 +103,7 @@ def _diff(
obj,
cache,
relink=False,
dvcignore: Optional[DvcIgnoreFilter] = None,
ignore: Optional["Ignore"] = None,
):
old = None
try:
@@ -111,7 +113,7 @@ def _diff(
fs,
obj.hash_info.name if obj else cache.fs.PARAM_CHECKSUM,
dry_run=True,
dvcignore=dvcignore,
ignore=ignore,
)
except FileNotFoundError:
pass
@@ -220,7 +222,7 @@ def checkout(
progress_callback=None,
relink=False,
quiet=False,
dvcignore: Optional[DvcIgnoreFilter] = None,
ignore: Optional["Ignore"] = None,
state=None,
):
# if protocol(fs_path) not in ["local", cache.fs.protocol]:
@@ -232,7 +234,7 @@ def checkout(
obj,
cache,
relink=relink,
dvcignore=dvcignore,
ignore=ignore,
)
failed = []
dvc/data/stage.py
+23 -7
View File
@@ -6,8 +6,6 @@ from concurrent.futures import ThreadPoolExecutor
from functools import partial
from typing import TYPE_CHECKING, Dict, Optional, Tuple
from dvc.exceptions import DvcIgnoreInCollectedDirError
from dvc.ignore import DvcIgnore
from dvc.objects.file import HashFile
from dvc.objects.hash import hash_file
from dvc.objects.hash_info import HashInfo
@@ -17,12 +15,24 @@ from dvc.progress import Tqdm
from .db.reference import ReferenceObjectDB
if TYPE_CHECKING:
from dvc.objects._ignore import Ignore
from dvc.objects.db import ObjectDB
from dvc.objects.fs.base import AnyFSPath, FileSystem
from .tree import Tree
DefaultIgnoreFile = ".dvcignore"
class IgnoreInCollectedDirError(Exception):
def __init__(self, ignore_file: str, ignore_dirname: str) -> None:
super().__init__(
f"{ignore_file} file should not be in collected dir path: "
f"'{ignore_dirname}'"
)
logger = logging.getLogger(__name__)
@@ -73,13 +83,13 @@ def _build_objects(
fs_path,
fs,
name,
dvcignore=None,
ignore: "Ignore" = None,
jobs=None,
no_progress_bar=False,
**kwargs,
):
if dvcignore:
walk_iterator = dvcignore.find(fs, fs_path)
if ignore:
walk_iterator = ignore.find(fs, fs_path)
else:
walk_iterator = fs.find(fs_path)
with Tqdm(
@@ -109,10 +119,16 @@ def _build_tree(fs_path, fs, name, **kwargs):
from .tree import Tree
tree_meta = Meta(size=0, nfiles=0)
# assuring mypy that they are not None but integer
assert tree_meta.size is not None
assert tree_meta.nfiles is not None
tree = Tree(None, None, None)
for file_fs_path, meta, obj in _iter_objects(fs_path, fs, name, **kwargs):
if DvcIgnore.DVCIGNORE_FILE == fs.path.name(file_fs_path):
raise DvcIgnoreInCollectedDirError(fs.path.parent(file_fs_path))
if fs.path.name(file_fs_path) == DefaultIgnoreFile:
raise IgnoreInCollectedDirError(
DefaultIgnoreFile, fs.path.parent(file_fs_path)
)
# NOTE: this is lossy transformation:
# "hey\there" -> "hey/there"
dvc/dependency/repo.py
+1 -1
View File
@@ -72,7 +72,7 @@ class RepoDependency(Dependency):
to.fs,
obj,
self.repo.odb.local,
dvcignore=None,
ignore=None,
state=self.repo.state,
)
dvc/exceptions.py
-8
View File
@@ -202,14 +202,6 @@ class FileMissingError(DvcException):
)
class DvcIgnoreInCollectedDirError(DvcException):
def __init__(self, ignore_dirname):
super().__init__(
".dvcignore file should not be in collected dir path: "
"'{}'".format(ignore_dirname)
)
class FileTransferError(DvcException):
_METHOD = "transfer"
dvc/objects/_ignore.py
+11
View File
@@ -0,0 +1,11 @@
from typing import TYPE_CHECKING, Iterator, Protocol
if TYPE_CHECKING:
from .fs.base import AnyFSPath, FileSystem
class Ignore(Protocol):
def find(
self, fs: "FileSystem", path: "AnyFSPath"
) -> Iterator["AnyFSPath"]:
...
dvc/objects/state.py
+15 -9
View File
@@ -3,6 +3,7 @@
import logging
import os
from abc import ABC, abstractmethod
from typing import TYPE_CHECKING
from .fs import LocalFileSystem
from .fs.system import inode as get_inode
@@ -10,6 +11,10 @@ from .fs.utils import relpath
from .hash_info import HashInfo
from .utils import get_mtime_and_size
if TYPE_CHECKING:
from ._ignore import Ignore
logger = logging.getLogger(__name__)
@@ -46,21 +51,22 @@ class StateNoop(StateBase):
class State(StateBase): # pylint: disable=too-many-instance-attributes
def __init__(self, root_dir=None, tmp_dir=None, dvcignore=None):
def __init__(self, root_dir=None, tmp_dir=None, ignore: "Ignore" = None):
from .cache import Cache
super().__init__()
self.tmp_dir = tmp_dir
self.root_dir = root_dir
self.dvcignore = dvcignore
self.ignore = ignore
if not tmp_dir:
return
config = {"eviction_policy": "least-recently-used"}
self.links = Cache(directory=os.path.join(tmp_dir, "links"), **config)
self.md5s = Cache(directory=os.path.join(tmp_dir, "md5s"), **config)
links_dir = os.path.join(tmp_dir, "links")
md5s_dir = os.path.join(tmp_dir, "md5s")
self.links = Cache(links_dir, eviction_policy="least-recently-used")
self.md5s = Cache(md5s_dir, eviction_policy="least-recently-used")
def close(self):
self.md5s.close()
@@ -77,7 +83,7 @@ class State(StateBase): # pylint: disable=too-many-instance-attributes
if not isinstance(fs, LocalFileSystem):
return
mtime, size = get_mtime_and_size(path, fs, self.dvcignore)
mtime, size = get_mtime_and_size(path, fs, self.ignore)
inode = get_inode(path)
logger.debug(
@@ -107,7 +113,7 @@ class State(StateBase): # pylint: disable=too-many-instance-attributes
return None, None
try:
mtime, size = get_mtime_and_size(path, fs, self.dvcignore)
mtime, size = get_mtime_and_size(path, fs, self.ignore)
except FileNotFoundError:
return None, None
@@ -131,7 +137,7 @@ class State(StateBase): # pylint: disable=too-many-instance-attributes
return
try:
mtime, _ = get_mtime_and_size(path, fs, self.dvcignore)
mtime, _ = get_mtime_and_size(path, fs, self.ignore)
except FileNotFoundError:
return
@@ -160,7 +166,7 @@ class State(StateBase): # pylint: disable=too-many-instance-attributes
continue
inode = get_inode(path)
mtime, _ = get_mtime_and_size(path, fs, self.dvcignore)
mtime, _ = get_mtime_and_size(path, fs, self.ignore)
if ref[relative_path] == (inode, mtime):
logger.debug("Removing '%s' as unused link.", path)
dvc/objects/utils.py
+4 -3
View File
@@ -4,11 +4,12 @@ import json
from typing import TYPE_CHECKING, Tuple
if TYPE_CHECKING:
from ._ignore import Ignore
from .fs.base import AnyFSPath, FileSystem
def get_mtime_and_size(
path: "AnyFSPath", fs: "FileSystem", dvcignore=None
path: "AnyFSPath", fs: "FileSystem", ignore: "Ignore" = None
) -> Tuple[str, int]:
import nanotime
@@ -20,8 +21,8 @@ def get_mtime_and_size(
size = 0
files_mtimes = {}
if dvcignore:
walk_iterator = dvcignore.find(fs, path)
if ignore:
walk_iterator = ignore.find(fs, path)
else:
walk_iterator = fs.find(path)
for file_path in walk_iterator:
dvc/output.py
+6 -6
View File
@@ -430,7 +430,7 @@ class Output:
self.fs_path,
self.fs,
name,
dvcignore=self.dvcignore,
ignore=self.dvcignore,
dry_run=not self.use_cache,
)
return obj.hash_info
@@ -552,7 +552,7 @@ class Output:
self.fs_path,
self.fs,
self.fs.PARAM_CHECKSUM,
dvcignore=self.dvcignore,
ignore=self.dvcignore,
dry_run=True,
)
self.hash_info = obj.hash_info
@@ -569,7 +569,7 @@ class Output:
self.fs_path,
self.fs,
self.odb.fs.PARAM_CHECKSUM,
dvcignore=self.dvcignore,
ignore=self.dvcignore,
)
self.hash_info = self.obj.hash_info
@@ -597,7 +597,7 @@ class Output:
filter_info or self.fs_path,
self.fs,
self.odb.fs.PARAM_CHECKSUM,
dvcignore=self.dvcignore,
ignore=self.dvcignore,
)
otransfer(
staging,
@@ -612,7 +612,7 @@ class Output:
obj,
self.odb,
relink=True,
dvcignore=self.dvcignore,
ignore=self.dvcignore,
state=self.repo.state,
)
self.set_exec()
@@ -626,7 +626,7 @@ class Output:
self.fs_path,
self.fs,
self.odb.fs.PARAM_CHECKSUM,
dvcignore=self.dvcignore,
ignore=self.dvcignore,
)
save_obj = save_obj.filter(prefix)
checkout_obj = save_obj.get(self.odb, prefix)
dvc/repo/diff.py
+1 -1
View File
@@ -142,7 +142,7 @@ def _output_paths(repo, targets):
repo.odb.local.fs,
"md5",
dry_run=True,
dvcignore=output.dvcignore,
ignore=output.dvcignore,
)
hash_info = obj.hash_info
else:
tests/func/test_external_repo.py
+1 -1
View File
@@ -209,7 +209,7 @@ def test_subrepos_are_ignored(tmp_dir, erepo_dir):
"dir",
repo.dvcfs,
"md5",
dvcignore=repo.dvcignore,
ignore=repo.dvcignore,
)
transfer(
staging,
tests/func/test_ignore.py
+2 -2
View File
@@ -4,7 +4,7 @@ from pathlib import Path
import pytest
from dvc.exceptions import DvcIgnoreInCollectedDirError
from dvc.data.stage import IgnoreInCollectedDirError
from dvc.ignore import DvcIgnore, DvcIgnorePatterns
from dvc.objects.utils import get_mtime_and_size
from dvc.output import OutputIsIgnoredError
@@ -87,7 +87,7 @@ def test_remove_file(tmp_dir, dvc):
def test_dvcignore_in_out_dir(tmp_dir, dvc):
tmp_dir.gen({"dir": {"foo": "foo", DvcIgnore.DVCIGNORE_FILE: ""}})
with pytest.raises(DvcIgnoreInCollectedDirError):
with pytest.raises(IgnoreInCollectedDirError):
dvc.add("dir")
tests/unit/cli/test_main.py
+16
View File
@@ -4,6 +4,7 @@ import pytest
from funcy import raiser
from dvc.cli import main
from dvc.data.stage import IgnoreInCollectedDirError
from dvc.objects.cache import DiskError
from dvc.objects.fs.base import FileSystem, RemoteMissingDepsError
@@ -68,3 +69,18 @@ def test_remote_missing_deps_are_correctly_reported(
"Thank you!"
)
assert expected in caplog.text
def test_ignore_in_collected_dir_error_is_logged(tmp_dir, caplog, mocker):
error = IgnoreInCollectedDirError(".dvcignore", "dir")
mocker.patch(
"dvc.cli.parse_args",
return_value=Namespace(
func=raiser(error),
quiet=False,
verbose=True,
),
)
assert main() == 255
expected = ".dvcignore file should not be in collected dir path: 'dir'"
assert expected in caplog.text