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
4 changes: 4 additions & 0 deletions dvc/command/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ def __init__(self, args):

class CmdRemoteAdd(CmdRemoteConfig):
def run(self):
if self.args.default:
logger.info(
"Setting '{}' as a default remote.".format(self.args.name)
)
self.remote_config.add(
self.args.name,
self.args.url,
Expand Down
35 changes: 34 additions & 1 deletion dvc/external_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@

from funcy import retry

from dvc.config import NoRemoteError
from dvc.config import NoRemoteError, ConfigError
from dvc.exceptions import NoRemoteInExternalRepoError
from dvc.remote import RemoteConfig
from dvc.exceptions import NoOutputInExternalRepoError
from dvc.exceptions import OutputNotFoundError
from dvc.utils.fs import remove
Expand Down Expand Up @@ -69,6 +70,22 @@ def _external_repo(url=None, rev=None, cache_dir=None):

repo = Repo(new_path)
try:
# check if the URL is local and no default remote is present
# add default remote pointing to the original repo's cache location
if os.path.isdir(url):
rconfig = RemoteConfig(repo.config)
if not _default_remote_set(rconfig):
Comment on lines +73 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maykulkarni @efiop does this logic mean that if there's a default remote setup, still DVC skips the local cache? Because I think it should try the local cache first regardless of remotes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jorgeorpinel Yes, precisely. It is hard for us to try both right now, as we don't have that kind of cascading remote logic implemented. I would wait for someone to ask for this.

Copy link
Contributor

@jorgeorpinel jorgeorpinel Dec 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm not sure I understand:

  1. Every DVC repo has a cache directory right? So in theory it can always be the first place to get stuff from when using get/import from another local project. Why does this require complicated remote checking logic?

  2. Does this PR really close import: get from source project cache if url/path are in the local systemΒ #2599 per that issue's description and discussion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Yes, but "first place to get stuff from" means that there is a second place, and our code is not ready to cascade from first to second yet, if it can't find all the stuff it needs in the local cache dir.

  2. From what we've understood initially - yes, but now it looks like we didn't quite understand each other.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. OK!

original_repo = Repo(url)
try:
rconfig.add(
"auto-generated-upstream",
original_repo.cache.local.cache_dir,
default=True,
level=Config.LEVEL_LOCAL,
)
finally:
original_repo.close()

if cache_dir is not None:
cache_config = CacheConfig(repo.config)
cache_config.set_dir(cache_dir, level=Config.LEVEL_LOCAL)
Expand Down Expand Up @@ -113,3 +130,19 @@ def _clone_repo(url, path):

git = Git.clone(url, path)
git.close()


def _default_remote_set(rconfig):
"""
Checks if default remote config is present.
Args:
rconfig: a remote config

Returns:
True if the default remote config is set, else False
"""
try:
rconfig.get_default()
return True
except ConfigError:
return False
1 change: 0 additions & 1 deletion dvc/remote/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ def add(self, name, url, default=False, force=False, level=None):
level=level,
)
if default:
logger.info("Setting '{}' as a default remote.".format(name))
self.config.set(
Config.SECTION_CORE,
Config.SECTION_CORE_REMOTE,
Expand Down
6 changes: 0 additions & 6 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from git import Repo
from git.exc import GitCommandNotFound

from dvc.remote.config import RemoteConfig
from dvc.remote.ssh.connection import SSHConnection
from dvc.repo import Repo as DvcRepo
from dvc.utils.compat import cast_bytes_py2
Expand Down Expand Up @@ -148,11 +147,6 @@ def erepo(repo_dir):
repo.dvc.scm.add([stage_foo.path, stage_bar.path, stage_data_dir.path])
repo.dvc.scm.commit("init repo")

rconfig = RemoteConfig(repo.dvc.config)
rconfig.add("upstream", repo.dvc.cache.local.cache_dir, default=True)
repo.dvc.scm.add([repo.dvc.config.config_file])
repo.dvc.scm.commit("add remote")

repo.create("version", "master")
repo.dvc.add("version")
repo.dvc.scm.add([".gitignore", "version.dvc"])
Expand Down