Repository: iterative/dvc (python)

Issue:
import-url/get-url: support smth like --config
To be able to pass remote config options on the fly. Similar to `--remote-config` in `import`.

Need to take care figuring out a proper name for this, because we already have `--config` and `--remote-config` elsewhere. So maybe `--url-config`?

Pull request:
ls-url/get-url/import-url: introduce --fs-config
Useful when you want to set additional options for your url. E.g. `account_name` for azure, or `profile` for aws.

Considering that we already have `dvcfs` thing in API, I think it is time to embrace `fs` concept and document it.

Fixes #9886


Patch:
diff --git a/dvc/commands/get_url.py b/dvc/commands/get_url.py
index e96deb50e..9270f57a3 100644
--- a/dvc/commands/get_url.py
+++ b/dvc/commands/get_url.py
@@ -3,7 +3,7 @@ import logging
 
 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__)
@@ -19,6 +19,7 @@ class CmdGetUrl(CmdBaseNoRepo):
                 out=self.args.out,
                 jobs=self.args.jobs,
                 force=self.args.force,
+                fs_config=self.args.fs_config,
             )
             return 0
         except DvcException:
@@ -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)
diff --git a/dvc/commands/imp_url.py b/dvc/commands/imp_url.py
index 21d826c57..039c34545 100644
--- a/dvc/commands/imp_url.py
+++ b/dvc/commands/imp_url.py
@@ -3,7 +3,7 @@ import logging
 
 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__)
@@ -22,6 +22,7 @@ class CmdImportUrl(CmdBase):
                 jobs=self.args.jobs,
                 force=self.args.force,
                 version_aware=self.args.version_aware,
+                fs_config=self.args.fs_config,
             )
         except DvcException:
             logger.exception(
@@ -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)
diff --git a/dvc/commands/ls_url.py b/dvc/commands/ls_url.py
index 80034f549..fbbdd86d0 100644
--- a/dvc/commands/ls_url.py
+++ b/dvc/commands/ls_url.py
@@ -2,7 +2,7 @@ import argparse
 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
 
@@ -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
@@ -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)
diff --git a/dvc/dependency/__init__.py b/dvc/dependency/__init__.py
index bf2c64204..2904da8a8 100644
--- a/dvc/dependency/__init__.py
+++ b/dvc/dependency/__init__.py
@@ -15,6 +15,7 @@ SCHEMA: Mapping[str, Any] = {
     **ARTIFACT_SCHEMA,
     **RepoDependency.REPO_SCHEMA,
     Output.PARAM_FILES: [DIR_FILES_SCHEMA],
+    Output.PARAM_FS_CONFIG: dict,
 }
 
 
@@ -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
 
 
diff --git a/dvc/output.py b/dvc/output.py
index ae8b303fb..5cf213300 100644
--- a/dvc/output.py
+++ b/dvc/output.py
@@ -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,
@@ -111,6 +112,7 @@ def loadd_from(stage, d_list):
                 files=files,
                 push=push,
                 hash_name=hash_name,
+                fs_config=fs_config,
             )
         )
     return ret
@@ -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
@@ -351,6 +354,7 @@ class Output:
         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)
 
@@ -872,6 +876,9 @@ class Output:
 
         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:
@@ -1537,4 +1544,5 @@ SCHEMA = {
     Output.PARAM_REMOTE: str,
     Output.PARAM_PUSH: bool,
     Output.PARAM_FILES: [DIR_FILES_SCHEMA],
+    Output.PARAM_FS_CONFIG: dict,
 }
diff --git a/dvc/repo/get_url.py b/dvc/repo/get_url.py
index 7b6ab4f8f..ff0e57cf4 100644
--- a/dvc/repo/get_url.py
+++ b/dvc/repo/get_url.py
@@ -6,12 +6,12 @@ from dvc.fs import download, parse_external_url
 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)
diff --git a/dvc/repo/ls_url.py b/dvc/repo/ls_url.py
index 496daa128..bae2e9ff3 100644
--- a/dvc/repo/ls_url.py
+++ b/dvc/repo/ls_url.py
@@ -2,8 +2,8 @@ from dvc.exceptions import URLMissingError
 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:


Test patch:
diff --git a/dvc/testing/workspace_tests.py b/dvc/testing/workspace_tests.py
index 216c83ff1..e99860114 100644
--- a/dvc/testing/workspace_tests.py
+++ b/dvc/testing/workspace_tests.py
@@ -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,
@@ -185,7 +185,7 @@ class TestLsUrl:
         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,
@@ -200,7 +200,7 @@ class TestLsUrl:
         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,
@@ -212,14 +212,14 @@ class TestLsUrl:
 
     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"
@@ -229,7 +229,7 @@ class TestGetUrl:
         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()
@@ -242,14 +242,14 @@ class TestGetUrl:
             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:
diff --git a/tests/func/test_import_url.py b/tests/func/test_import_url.py
index 02eecbbfd..e8ae45d72 100644
--- a/tests/func/test_import_url.py
+++ b/tests/func/test_import_url.py
@@ -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}
diff --git a/tests/unit/command/test_get_url.py b/tests/unit/command/test_get_url.py
index a2b27495b..617940f15 100644
--- a/tests/unit/command/test_get_url.py
+++ b/tests/unit/command/test_get_url.py
@@ -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)
diff --git a/tests/unit/command/test_imp_url.py b/tests/unit/command/test_imp_url.py
index 8944cd4c2..3804f988e 100644
--- a/tests/unit/command/test_imp_url.py
+++ b/tests/unit/command/test_imp_url.py
@@ -34,6 +34,7 @@ def test_import_url(mocker, dvc):
         jobs=4,
         force=False,
         version_aware=False,
+        fs_config=None,
     )
 
 
@@ -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,
     )
 
@@ -115,6 +117,7 @@ def test_import_url_to_remote(mocker, dvc):
         jobs=None,
         force=False,
         version_aware=False,
+        fs_config=None,
     )
 
 
diff --git a/tests/unit/command/test_ls_url.py b/tests/unit/command/test_ls_url.py
index 84f341856..20e2676bf 100644
--- a/tests/unit/command/test_ls_url.py
+++ b/tests/unit/command/test_ls_url.py
@@ -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):
@@ -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)