From df67dc2db8e5de5020726002b5c4c3f4acfcddae Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Fri, 11 Jun 2021 19:52:42 +0545 Subject: [PATCH] webdav: bring back support for user in url (#6155) Also added some optimizations, and merged Webdavs into the same file. --- dvc/fs/__init__.py | 7 +++--- dvc/fs/webdav.py | 28 ++++++++++++++--------- dvc/fs/webdavs.py | 7 ------ setup.py | 2 +- tests/unit/remote/test_webdav.py | 38 +++++++++++++++++++++++++------- 5 files changed, 51 insertions(+), 31 deletions(-) delete mode 100644 dvc/fs/webdavs.py diff --git a/dvc/fs/__init__.py b/dvc/fs/__init__.py index fe38415e2..117247d35 100644 --- a/dvc/fs/__init__.py +++ b/dvc/fs/__init__.py @@ -12,8 +12,7 @@ from .local import LocalFileSystem from .oss import OSSFileSystem from .s3 import S3FileSystem from .ssh import SSHFileSystem -from .webdav import WebDAVFileSystem -from .webdavs import WebDAVSFileSystem +from .webdav import WebDAVFileSystem, WebDAVSFileSystem from .webhdfs import WebHDFSFileSystem FS_MAP = { @@ -112,5 +111,5 @@ def get_cloud_fs(repo, **kwargs): ) extras = cls._get_kwargs_from_urls(url) # pylint:disable=protected-access - - return cls, dict(**extras, **remote_conf), path_info + conf = {**extras, **remote_conf} # remote config takes priority + return cls, conf, path_info diff --git a/dvc/fs/webdav.py b/dvc/fs/webdav.py index 30327dc25..547b12803 100644 --- a/dvc/fs/webdav.py +++ b/dvc/fs/webdav.py @@ -20,7 +20,6 @@ class WebDAVFileSystem(FSSpecWrapper): # pylint:disable=abstract-method CAN_TRAVERSE = True TRAVERSE_PREFIX_LEN = 2 REQUIRES = {"webdav4": "webdav4"} - CHUNK_SIZE = 2 ** 16 PARAM_CHECKSUM = "etag" DETAIL_FIELDS = frozenset(("etag", "size")) @@ -29,13 +28,15 @@ class WebDAVFileSystem(FSSpecWrapper): # pylint:disable=abstract-method cert_path = config.get("cert_path", None) key_path = config.get("key_path", None) - cert = cert_path if not key_path else (cert_path, key_path) - - ssl_verify = config.get("ssl_verify", True) - self.fs_args.update( - {"base_url": config["url"], "cert": cert, "verify": ssl_verify} - ) self.prefix = config.get("prefix", "") + self.fs_args.update( + { + "base_url": config["url"], + "cert": cert_path if not key_path else (cert_path, key_path), + "verify": config.get("ssl_verify", True), + "timeout": config.get("timeout", 30), + } + ) @staticmethod def _get_kwargs_from_urls(urlpath): @@ -44,6 +45,7 @@ class WebDAVFileSystem(FSSpecWrapper): # pylint:disable=abstract-method "prefix": path_info.path.rstrip("/"), "host": path_info.replace(path="").url, "url": path_info.url.rstrip("/"), + "user": path_info.user, } def _prepare_credentials(self, **config): @@ -70,10 +72,10 @@ class WebDAVFileSystem(FSSpecWrapper): # pylint:disable=abstract-method def _upload_fobj(self, fobj, to_info): rpath = self.translate_path_info(to_info) self.makedirs(os.path.dirname(rpath)) - # using upload_fileobj to directly upload fileobj - # rather than buffering it. - # TODO: retry upload on failure - return self.fs.client.upload_fileobj(fobj, rpath) + # using upload_fileobj to directly upload fileobj rather than buffering + # and using overwrite=True to avoid check for an extra exists call, + # as caller should ensure that the file does not exist beforehand. + return self.fs.client.upload_fileobj(fobj, rpath, overwrite=True) def makedirs(self, path_info): path = self.translate_path_info(path_info) @@ -89,3 +91,7 @@ class WebDAVFileSystem(FSSpecWrapper): # pylint:disable=abstract-method def _strip_bucket(self, entry): return entry + + +class WebDAVSFileSystem(WebDAVFileSystem): # pylint:disable=abstract-method + scheme = Schemes.WEBDAVS diff --git a/dvc/fs/webdavs.py b/dvc/fs/webdavs.py deleted file mode 100644 index 984ea5fe5..000000000 --- a/dvc/fs/webdavs.py +++ /dev/null @@ -1,7 +0,0 @@ -from dvc.scheme import Schemes - -from .webdav import WebDAVFileSystem - - -class WebDAVSFileSystem(WebDAVFileSystem): # pylint:disable=abstract-method - scheme = Schemes.WEBDAVS diff --git a/setup.py b/setup.py index abe602f57..b0423aa4f 100644 --- a/setup.py +++ b/setup.py @@ -106,7 +106,7 @@ ssh = ["paramiko[invoke]>=2.7.0"] # Remove the env marker if/when pyarrow is available for Python3.9 hdfs = ["pyarrow>=2.0.0"] webhdfs = ["hdfs==2.5.8"] -webdav = ["webdav4>=0.6.1"] +webdav = ["webdav4>=0.7.0"] # gssapi should not be included in all_remotes, because it doesn't have wheels # for linux and mac, so it will fail to compile if user doesn't have all the # requirements, including kerberos itself. Once all the wheels are available, diff --git a/tests/unit/remote/test_webdav.py b/tests/unit/remote/test_webdav.py index 0e9ea5553..70a283b3f 100644 --- a/tests/unit/remote/test_webdav.py +++ b/tests/unit/remote/test_webdav.py @@ -1,29 +1,51 @@ -from dvc.fs.webdav import WebDAVFileSystem +import pytest + +from dvc.fs import get_cloud_fs +from dvc.fs.webdav import WebDAVFileSystem, WebDAVSFileSystem # Test configuration -url = "webdavs://example.com/public.php/webdav" +url = "webdav://example.com/public.php/webdav" user = "username" -userurl = f"webdavs://{user}@example.com/public.php/webdav" +userurl = f"webdav://{user}@example.com/public.php/webdav" password = "password" +https_url = "webdavs://example.com/public.php/webdav" -def test_user(dvc): +def test_user(): fs = WebDAVFileSystem(url=url, user=user) assert fs.user == user -def test_password(dvc): +def test_password(): config = {"url": url, "user": user, "password": password} fs = WebDAVFileSystem(**config) assert fs.password == password -def test_ssl_verify_custom_cert(dvc): +def test_ssl_verify_custom_cert(): config = { - "url": "http://example.com/", - "path_info": "file.html", + "url": url, "ssl_verify": "/path/to/custom/cabundle.pem", } fs = WebDAVFileSystem(**config) assert fs.fs_args["verify"] == "/path/to/custom/cabundle.pem" + + +@pytest.mark.parametrize( + "base_url, fs_cls", + [(url, WebDAVFileSystem), (https_url, WebDAVSFileSystem)], +) +def test_remote_with_jobs(dvc, base_url, fs_cls): + remote_config = {"url": base_url, "user": user} + + dvc.config["remote"]["dav"] = remote_config + cls, config, _ = get_cloud_fs(dvc, name="dav") + assert config["user"] == user + assert cls is fs_cls + + # config from remote takes priority + remote_config.update({"user": "admin"}) + cls, config, _ = get_cloud_fs(dvc, name="dav") + assert config["user"] == "admin" + assert cls is fs_cls