get: leverage external_repo() context manager for DVC repositories

Author: fabiosantoscode

dvc/command/get.py

@@ -32,7 +32,7 @@ def run(self):
 
 def add_parser(subparsers, parent_parser):
     GET_HELP = (
-        "Download a file or directory from any DVC project or Git repository"
+        "Download a file or directory from any DVC project or Git repository."
     )
     get_parser = subparsers.add_parser(
         "get",
@@ -43,7 +43,7 @@ def add_parser(subparsers, parent_parser):
     )
     get_parser.add_argument(
         "url",
-        help="Location of DVC project or Git repository to download from.",
+        help="Location of DVC project or Git repository to download from",
     )
     get_parser.add_argument(
         "path",
@@ -53,7 +53,7 @@ def add_parser(subparsers, parent_parser):
         "-o",
         "--out",
         nargs="?",
-        help="Destination path to copy/download files to.",
+        help="Destination path to download files to",
     )
     get_parser.add_argument(
         "--rev", nargs="?", help="Git revision (e.g. branch, tag, SHA)"

dvc/command/imp.py

@@ -30,7 +30,7 @@ def run(self):
 
 def add_parser(subparsers, parent_parser):
     IMPORT_HELP = (
-        "Download data from DVC project or Git repository and take it under "
+        "Download a file or directory from any DVC project or Git repository and take it under "
         "DVC control."
     )
 
@@ -43,13 +43,13 @@ def add_parser(subparsers, parent_parser):
     )
     import_parser.add_argument(
         "url",
-        help="Location of Git repository with DVC project to download from.",
+        help="Location of DVC project or Git repository to download from",
     )
-    import_parser.add_argument("path", help="Path to data within DVC project.")
+    import_parser.add_argument("path", help="Path to a file or directory within the project or repository")
     import_parser.add_argument(
-        "-o", "--out", nargs="?", help="Destination path to put data in."
+        "-o", "--out", nargs="?", help="Destination path to download files to"
     )
     import_parser.add_argument(
-        "--rev", nargs="?", help="DVC repository git revision."
+        "--rev", nargs="?", help="Git revision (e.g. branch, tag, SHA)"
     )
     import_parser.set_defaults(func=CmdImport)

dvc/external_repo.py

@@ -33,7 +33,7 @@ def external_repo(url=None, rev=None, rev_lock=None, cache_dir=None):
     repo.close()
 
 
-def cached_clone(url, rev=None, clone_path=None, **_ignored_kwargs):
+def cached_clone(url, rev=None, **_ignored_kwargs):
     """Clone an external git repo to a temporary directory.
 
     Returns the path to a local temporary directory with the specified
@@ -44,7 +44,7 @@ def cached_clone(url, rev=None, clone_path=None, **_ignored_kwargs):
 
     """
 
-    new_path = clone_path or tempfile.mkdtemp("dvc-erepo")
+    new_path = tempfile.mkdtemp("dvc-erepo")
 
     # Copy and adjust existing clean clone
     if (url, None, None) in REPO_CACHE:

dvc/repo/get.py

@@ -9,7 +9,7 @@
     OutputNotFoundError,
     PathMissingError,
 )
-from dvc.external_repo import cached_clone
+from dvc.external_repo import external_repo, cached_clone
 from dvc.path_info import PathInfo
 from dvc.stage import Stage
 from dvc.utils import resolve_output
@@ -29,8 +29,6 @@ def __init__(self):
 
 @staticmethod
 def get(url, path, out=None, rev=None):
-    from dvc.repo import Repo
-
     out = resolve_output(path, out)
 
     if Stage.is_valid_filename(out):
@@ -43,41 +41,50 @@ def get(url, path, out=None, rev=None):
     # and won't work with reflink/hardlink.
     dpath = os.path.dirname(os.path.abspath(out))
     tmp_dir = os.path.join(dpath, "." + str(shortuuid.uuid()))
+    raw_git_dir = None
     try:
-        cached_clone(url, rev=rev, clone_path=tmp_dir)
         try:
-            repo = Repo(tmp_dir)
-
-            # Try any links possible to avoid data duplication.
-            #
-            # Not using symlink, because we need to remove cache after we are
-            # done, and to make that work we would have to copy data over
-            # anyway before removing the cache, so we might just copy it
-            # right away.
-            #
-            # Also, we can't use theoretical "move" link type here, because
-            # the same cache file might be used a few times in a directory.
-            repo.cache.local.cache_types = ["reflink", "hardlink", "copy"]
-
-            output = repo.find_out_by_relpath(path)
-            if output.use_cache:
-                _get_cached(repo, output, out)
+            with external_repo(cache_dir=tmp_dir, url=url, rev=rev) as repo:
+                # Try any links possible to avoid data duplication.
+                #
+                # Not using symlink, because we need to remove cache after we
+                # are done, and to make that work we would have to copy data
+                # over anyway before removing the cache, so we might just copy
+                # it right away.
+                #
+                # Also, we can't use theoretical "move" link type here, because
+                # the same cache file might be used a few times in a directory.
+                repo.cache.local.cache_types = ["reflink", "hardlink", "copy"]
+
+                try:
+                    output = repo.find_out_by_relpath(path)
+                except OutputNotFoundError:
+                    output = None
+
+                if output and output.use_cache:
+                    _get_cached(repo, output, out)
+                    return
+
+                # Either an uncached out with absolute path or a user error
+
+                if os.path.isabs(path):
+                    raise FileNotFoundError
+
+                fs_copy(os.path.join(repo.root_dir, path), out)
                 return
 
-        except (NotDvcRepoError, OutputNotFoundError):
+        except NotDvcRepoError:
+            # Not a DVC repository, continue below and copy from git
             pass
 
-        # It's an uncached out with absolute path, a non-DVC repo, or a
-        # user error
-        if os.path.isabs(path):
-            raise FileNotFoundError
-
-        fs_copy(os.path.join(tmp_dir, path), out)
-
+        raw_git_dir = cached_clone(url, rev=rev)
+        fs_copy(os.path.join(raw_git_dir, path), out)
     except (OutputNotFoundError, FileNotFoundError):
         raise PathMissingError(path, url)
     finally:
         remove(tmp_dir)
+        if raw_git_dir:
+            remove(raw_git_dir)
 
 
 def _get_cached(repo, output, out):