Skip to content

Commit

Permalink
ls-url/get-url/import-url: introduce --fs-config
Browse files Browse the repository at this point in the history
  • Loading branch information
efiop committed Aug 30, 2023
1 parent 7a9da0f commit bc0a522
Show file tree
Hide file tree
Showing 12 changed files with 72 additions and 20 deletions.
10 changes: 9 additions & 1 deletion dvc/commands/get_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from dvc.cli import completion
from dvc.cli.command import CmdBaseNoRepo
from dvc.cli.utils import append_doc_link
from dvc.cli.utils import DictAction, append_doc_link
from dvc.exceptions import DvcException

logger = logging.getLogger(__name__)
Expand All @@ -19,6 +19,7 @@ def run(self):
out=self.args.out,
jobs=self.args.jobs,
force=self.args.force,
fs_config=self.args.fs_config,
)
return 0
except DvcException:
Expand Down Expand Up @@ -58,4 +59,11 @@ def add_parser(subparsers, parent_parser):
default=False,
help="Override local file or folder if exists.",
)
get_parser.add_argument(
"--fs-config",
type=str,
nargs="*",
action=DictAction,
help="Config options for the target url.",
)
get_parser.set_defaults(func=CmdGetUrl)
10 changes: 9 additions & 1 deletion dvc/commands/imp_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from dvc.cli import completion
from dvc.cli.command import CmdBase
from dvc.cli.utils import append_doc_link
from dvc.cli.utils import DictAction, append_doc_link
from dvc.exceptions import DvcException

logger = logging.getLogger(__name__)
Expand All @@ -22,6 +22,7 @@ def run(self):
jobs=self.args.jobs,
force=self.args.force,
version_aware=self.args.version_aware,
fs_config=self.args.fs_config,
)
except DvcException:
logger.exception(
Expand Down Expand Up @@ -114,4 +115,11 @@ def add_parser(subparsers, parent_parser):
default=False,
help="Import using cloud versioning. Implied if the URL contains a version ID.",
)
import_parser.add_argument(
"--fs-config",
type=str,
nargs="*",
action=DictAction,
help="Config options for the target url.",
)
import_parser.set_defaults(func=CmdImportUrl)
15 changes: 13 additions & 2 deletions dvc/commands/ls_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import logging

from dvc.cli.command import CmdBaseNoRepo
from dvc.cli.utils import append_doc_link
from dvc.cli.utils import DictAction, append_doc_link

from .ls import show_entries

Expand All @@ -13,7 +13,11 @@ class CmdListUrl(CmdBaseNoRepo):
def run(self):
from dvc.repo import Repo

entries = Repo.ls_url(self.args.url, recursive=self.args.recursive)
entries = Repo.ls_url(
self.args.url,
recursive=self.args.recursive,
fs_config=self.args.fs_config,
)
if entries:
show_entries(entries, with_color=True, with_size=self.args.size)
return 0
Expand Down Expand Up @@ -43,4 +47,11 @@ def add_parser(subparsers, parent_parser):
action="store_true",
help="Show sizes.",
)
lsurl_parser.add_argument(
"--fs-config",
type=str,
nargs="*",
action=DictAction,
help="Config options for the target url.",
)
lsurl_parser.set_defaults(func=CmdListUrl)
6 changes: 5 additions & 1 deletion dvc/dependency/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
**ARTIFACT_SCHEMA,
**RepoDependency.REPO_SCHEMA,
Output.PARAM_FILES: [DIR_FILES_SCHEMA],
Output.PARAM_FS_CONFIG: dict,
}


Expand All @@ -36,7 +37,10 @@ def loadd_from(stage, d_list):
p = d.pop(Output.PARAM_PATH, None)
files = d.pop(Output.PARAM_FILES, None)
hash_name = d.pop(Output.PARAM_HASH, None)
ret.append(_get(stage, p, d, files=files, hash_name=hash_name))
fs_config = d.pop(Output.PARAM_FS_CONFIG, None)
ret.append(
_get(stage, p, d, files=files, hash_name=hash_name, fs_config=fs_config)
)
return ret


Expand Down
8 changes: 8 additions & 0 deletions dvc/output.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ def loadd_from(stage, d_list):
files = d.pop(Output.PARAM_FILES, None)
push = d.pop(Output.PARAM_PUSH, True)
hash_name = d.pop(Output.PARAM_HASH, None)
fs_config = d.pop(Output.PARAM_FS_CONFIG, None)
ret.append(
_get(
stage,
Expand All @@ -111,6 +112,7 @@ def loadd_from(stage, d_list):
files=files,
push=push,
hash_name=hash_name,
fs_config=fs_config,
)
)
return ret
Expand Down Expand Up @@ -313,6 +315,7 @@ class Output:
PARAM_PUSH = "push"
PARAM_CLOUD = "cloud"
PARAM_HASH = "hash"
PARAM_FS_CONFIG = "fs_config"

DoesNotExistError: Type[DvcException] = OutputDoesNotExistError
IsNotFileOrDirError: Type[DvcException] = OutputIsNotFileOrDirError
Expand Down Expand Up @@ -351,6 +354,7 @@ def __init__( # noqa: PLR0913
if meta.version_id or files:
fs_kwargs["version_aware"] = True

self.def_fs_config = fs_config
if fs_config is not None:
fs_kwargs.update(**fs_config)

Expand Down Expand Up @@ -872,6 +876,9 @@ def dumpd(self, **kwargs): # noqa: C901, PLR0912

ret[self.PARAM_PATH] = path

if self.def_fs_config:
ret[self.PARAM_FS_CONFIG] = self.def_fs_config

if not self.IS_DEPENDENCY:
ret.update(self.annot.to_dict())
if not self.use_cache:
Expand Down Expand Up @@ -1537,4 +1544,5 @@ def _merge_dir_version_meta(self, other: "Output"):
Output.PARAM_REMOTE: str,
Output.PARAM_PUSH: bool,
Output.PARAM_FILES: [DIR_FILES_SCHEMA],
Output.PARAM_FS_CONFIG: dict,
}
4 changes: 2 additions & 2 deletions dvc/repo/get_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
from dvc.utils import resolve_output


def get_url(url, out=None, *, config=None, jobs=None, force=False):
def get_url(url, out=None, *, fs_config=None, jobs=None, force=False):
out = resolve_output(url, out, force=force)
out = os.path.abspath(out)
(out,) = output.loads_from(None, [out], use_cache=False)

src_fs, src_path = parse_external_url(url, config)
src_fs, src_path = parse_external_url(url, fs_config)
if not src_fs.exists(src_path):
raise URLMissingError(url)
download(src_fs, src_path, out.fs_path, jobs=jobs)
4 changes: 2 additions & 2 deletions dvc/repo/ls_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
from dvc.fs import parse_external_url


def ls_url(url, *, config=None, recursive=False):
fs, fs_path = parse_external_url(url, config=config)
def ls_url(url, *, fs_config=None, recursive=False):
fs, fs_path = parse_external_url(url, config=fs_config)
try:
info = fs.info(fs_path)
except FileNotFoundError as exc:
Expand Down
16 changes: 8 additions & 8 deletions dvc/testing/workspace_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class TestLsUrl:
def test_file(self, cloud, fname):
cloud.gen({fname: "foo contents"})
fs, fs_path = parse_external_url(cloud.url, cloud.config)
result = ls_url(str(cloud / fname), config=cloud.config)
result = ls_url(str(cloud / fname), fs_config=cloud.config)
match_files(
fs,
result,
Expand All @@ -185,7 +185,7 @@ def test_dir(self, cloud):
if not (cloud / "dir").is_dir():
pytest.skip("Cannot create directories on this cloud")
fs, _ = parse_external_url(cloud.url, cloud.config)
result = ls_url(str(cloud / "dir"), config=cloud.config)
result = ls_url(str(cloud / "dir"), fs_config=cloud.config)
match_files(
fs,
result,
Expand All @@ -200,7 +200,7 @@ def test_recursive(self, cloud):
if not (cloud / "dir").is_dir():
pytest.skip("Cannot create directories on this cloud")
fs, _ = parse_external_url(cloud.url, cloud.config)
result = ls_url(str(cloud / "dir"), config=cloud.config, recursive=True)
result = ls_url(str(cloud / "dir"), fs_config=cloud.config, recursive=True)
match_files(
fs,
result,
Expand All @@ -212,14 +212,14 @@ def test_recursive(self, cloud):

def test_nonexistent(self, cloud):
with pytest.raises(URLMissingError):
ls_url(str(cloud / "dir"), config=cloud.config)
ls_url(str(cloud / "dir"), fs_config=cloud.config)


class TestGetUrl:
def test_get_file(self, cloud, tmp_dir):
cloud.gen({"foo": "foo contents"})

Repo.get_url(str(cloud / "foo"), "foo_imported", config=cloud.config)
Repo.get_url(str(cloud / "foo"), "foo_imported", fs_config=cloud.config)

assert (tmp_dir / "foo_imported").is_file()
assert (tmp_dir / "foo_imported").read_text() == "foo contents"
Expand All @@ -229,7 +229,7 @@ def test_get_dir(self, cloud, tmp_dir):
if not (cloud / "foo").is_dir():
pytest.skip("Cannot create directories on this cloud")

Repo.get_url(str(cloud / "foo"), "foo_imported", config=cloud.config)
Repo.get_url(str(cloud / "foo"), "foo_imported", fs_config=cloud.config)

assert (tmp_dir / "foo_imported").is_dir()
assert (tmp_dir / "foo_imported" / "foo").is_file()
Expand All @@ -242,14 +242,14 @@ def test_get_url_to_dir(self, cloud, tmp_dir, dname):
pytest.skip("Cannot create directories on this cloud")
tmp_dir.gen({"dir": {"subdir": {}}})

Repo.get_url(str(cloud / "src" / "foo"), dname, config=cloud.config)
Repo.get_url(str(cloud / "src" / "foo"), dname, fs_config=cloud.config)

assert (tmp_dir / dname).is_dir()
assert (tmp_dir / dname / "foo").read_text() == "foo contents"

def test_get_url_nonexistent(self, cloud):
with pytest.raises(URLMissingError):
Repo.get_url(str(cloud / "nonexistent"), config=cloud.config)
Repo.get_url(str(cloud / "nonexistent"), fs_config=cloud.config)


class TestToRemote:
Expand Down
10 changes: 10 additions & 0 deletions tests/func/test_import_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,19 @@ def test_import_url_fs_config(tmp_dir, dvc, workspace, mocker):
dep_init = mocker.spy(Dependency, "__init__")
dvc.imp_url(url, fs_config={"jobs": 42})

stage = load_file(dvc, "foo.dvc").stage
assert stage.deps[0].def_fs_config == {"jobs": 42}

dep_init_kwargs = dep_init.call_args[1]
assert dep_init_kwargs.get("fs_config") == {"jobs": 42}

assert get_fs_config.call_args_list[0][1] == {"url": "foo"}
assert get_fs_config.call_args_list[1][1] == {"url": url, "jobs": 42}
assert get_fs_config.call_args_list[2][1] == {"name": "workspace"}

dep_init.reset_mock()

dvc.pull("foo.dvc")

dep_init_kwargs = dep_init.call_args[1]
assert dep_init_kwargs.get("fs_config") == {"jobs": 42}
2 changes: 1 addition & 1 deletion tests/unit/command/test_get_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ def test_get_url(mocker):

assert cmd.run() == 0

m.assert_called_once_with("src", out="out", jobs=5, force=False)
m.assert_called_once_with("src", out="out", jobs=5, force=False, fs_config=None)
3 changes: 3 additions & 0 deletions tests/unit/command/test_imp_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def test_import_url(mocker, dvc):
jobs=4,
force=False,
version_aware=False,
fs_config=None,
)


Expand Down Expand Up @@ -83,6 +84,7 @@ def test_import_url_no_exec_download_flags(mocker, flag, expected, dvc):
jobs=None,
force=False,
version_aware=False,
fs_config=None,
**expected,
)

Expand Down Expand Up @@ -115,6 +117,7 @@ def test_import_url_to_remote(mocker, dvc):
jobs=None,
force=False,
version_aware=False,
fs_config=None,
)


Expand Down
4 changes: 2 additions & 2 deletions tests/unit/command/test_ls_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def test_ls_url(mocker):

assert cmd.run() == 0

m.assert_called_once_with("src", recursive=False)
m.assert_called_once_with("src", recursive=False, fs_config=None)


def test_recursive(mocker):
Expand All @@ -21,4 +21,4 @@ def test_recursive(mocker):

assert cmd.run() == 0

m.assert_called_once_with("src", recursive=True)
m.assert_called_once_with("src", recursive=True, fs_config=None)

0 comments on commit bc0a522

Please sign in to comment.