From 3ebccda62cc447c28e1068badcfc1ee4fce9148f Mon Sep 17 00:00:00 2001 From: Batuhan Taskaya Date: Tue, 2 Mar 2021 13:43:27 +0300 Subject: [PATCH] gs: use etag for the checksum parameter (#5534) Switch from `md5` to `etag` as the checksum parameter. Due to `etag` is not preserved while doing server side copies, this patch drops support for external gs outputs. --- dvc/dependency/gs.py | 8 +++++--- dvc/fs/gs.py | 7 +++---- dvc/output/__init__.py | 4 +--- dvc/output/gs.py | 7 ------- tests/func/test_add.py | 6 ------ tests/func/test_gc.py | 1 - tests/func/test_import_url.py | 5 ----- tests/func/test_output.py | 1 - tests/func/test_run_multistage.py | 6 ------ tests/unit/output/test_gs.py | 7 ------- 10 files changed, 9 insertions(+), 43 deletions(-) delete mode 100644 dvc/output/gs.py delete mode 100644 tests/unit/output/test_gs.py diff --git a/dvc/dependency/gs.py b/dvc/dependency/gs.py index 14dc5afd4..ca75749af 100644 --- a/dvc/dependency/gs.py +++ b/dvc/dependency/gs.py @@ -1,6 +1,8 @@ from dvc.dependency.base import BaseDependency -from dvc.output.gs import GSOutput +from dvc.output.base import BaseOutput + +from ..fs.gs import GSFileSystem -class GSDependency(BaseDependency, GSOutput): - pass +class GSDependency(BaseDependency, BaseOutput): + FS_CLS = GSFileSystem diff --git a/dvc/fs/gs.py b/dvc/fs/gs.py index 87a4a9637..74ba3d95e 100644 --- a/dvc/fs/gs.py +++ b/dvc/fs/gs.py @@ -13,8 +13,8 @@ class GSFileSystem(FSSpecWrapper): scheme = Schemes.GS PATH_CLS = CloudURLInfo REQUIRES = {"gcsfs": "gcsfs"} - PARAM_CHECKSUM = "md5" - DETAIL_FIELDS = frozenset(("md5", "size")) + PARAM_CHECKSUM = "etag" + DETAIL_FIELDS = frozenset(("etag", "size")) def __init__(self, repo, config): super().__init__(repo, config) @@ -31,8 +31,7 @@ class GSFileSystem(FSSpecWrapper): return login_info def _entry_hook(self, entry): - if "md5Hash" in entry: - entry["md5"] = base64.b64decode(entry["md5Hash"]).hex() + entry["etag"] = base64.b64decode(entry["etag"]).hex() return entry @wrap_prop(threading.Lock()) diff --git a/dvc/output/__init__.py b/dvc/output/__init__.py index 3154299e3..55c322d84 100644 --- a/dvc/output/__init__.py +++ b/dvc/output/__init__.py @@ -6,7 +6,6 @@ from voluptuous import And, Any, Coerce, Length, Lower, Required, SetTo from dvc.hash_info import HashInfo from dvc.output.base import BaseOutput -from dvc.output.gs import GSOutput from dvc.output.hdfs import HDFSOutput from dvc.output.local import LocalOutput from dvc.output.s3 import S3Output @@ -23,7 +22,6 @@ from ..fs.webhdfs import WebHDFSFileSystem OUTS_MAP = { Schemes.HDFS: HDFSOutput, Schemes.S3: S3Output, - Schemes.GS: GSOutput, Schemes.SSH: SSHOutput, Schemes.LOCAL: LocalOutput, Schemes.WEBHDFS: WebHDFSOutput, @@ -37,7 +35,7 @@ CHECKSUM_SCHEMA = Any( # NOTE: currently there are only 3 possible checksum names: # -# 1) md5 (LOCAL, SSH, GS); +# 1) md5 (LOCAL, SSH); # 2) etag (S3); # 3) checksum (HDFS); # diff --git a/dvc/output/gs.py b/dvc/output/gs.py deleted file mode 100644 index 86adcd8ed..000000000 --- a/dvc/output/gs.py +++ /dev/null @@ -1,7 +0,0 @@ -from dvc.output.base import BaseOutput - -from ..fs.gs import GSFileSystem - - -class GSOutput(BaseOutput): - FS_CLS = GSFileSystem diff --git a/tests/func/test_add.py b/tests/func/test_add.py index 0ac536dfc..a37ca0770 100644 --- a/tests/func/test_add.py +++ b/tests/func/test_add.py @@ -312,7 +312,6 @@ def test_add_filtered_files_in_dir( "etag", "8c7dd922ad47494fc02c388e12c00eac", ), - (pytest.lazy_fixture("gs"), "md5", "8c7dd922ad47494fc02c388e12c00eac"), ( pytest.lazy_fixture("hdfs"), "checksum", @@ -368,11 +367,6 @@ def test_add_external_file(tmp_dir, dvc, workspace, hash_name, hash_value): "etag", "ec602a6ba97b2dd07bd6d2cd89674a60.dir", ), - ( - pytest.lazy_fixture("gs"), - "md5", - "b6dcab6ccd17ca0a8bf4a215a37d14cc.dir", - ), ], indirect=["workspace"], ) diff --git a/tests/func/test_gc.py b/tests/func/test_gc.py index c8f6521ca..942858cfe 100644 --- a/tests/func/test_gc.py +++ b/tests/func/test_gc.py @@ -356,7 +356,6 @@ def test_gc_not_collect_pipeline_tracked_files(tmp_dir, dvc, run_copy): [ pytest.lazy_fixture("local_cloud"), pytest.lazy_fixture("s3"), - pytest.lazy_fixture("gs"), pytest.lazy_fixture("hdfs"), pytest.lazy_fixture("webhdfs"), pytest.param( diff --git a/tests/func/test_import_url.py b/tests/func/test_import_url.py index c515cd505..738b05521 100644 --- a/tests/func/test_import_url.py +++ b/tests/func/test_import_url.py @@ -158,11 +158,6 @@ def test_import_url(tmp_dir, dvc, workspace): "2aa17f8daa26996b3f7a4cf8888ac9ac", "ec602a6ba97b2dd07bd6d2cd89674a60.dir", ), - ( - pytest.lazy_fixture("gs"), - "dc24e1271084ee317ac3c2656fb8812b", - "b6dcab6ccd17ca0a8bf4a215a37d14cc.dir", - ), ( pytest.lazy_fixture("hdfs"), "ec0943f83357f702033c98e70b853c8c", diff --git a/tests/func/test_output.py b/tests/func/test_output.py index 3edbb4361..1ca7da832 100644 --- a/tests/func/test_output.py +++ b/tests/func/test_output.py @@ -6,7 +6,6 @@ from dvc.stage import Stage MARKERS = [pytest.mark.hdfs] TESTS = [ ("s3://bucket/path", "s3"), - ("gs://bucket/path", "gs"), ("ssh://example.com:/dir/path", "ssh"), ("path/to/file", "local"), ("path\\to\\file", "local"), diff --git a/tests/func/test_run_multistage.py b/tests/func/test_run_multistage.py index 8157f0b14..3894d018a 100644 --- a/tests/func/test_run_multistage.py +++ b/tests/func/test_run_multistage.py @@ -377,12 +377,6 @@ def test_run_overwrite_preserves_meta_and_comment(tmp_dir, dvc, run_copy): "acbd18db4cc2f85cedef654fccc4a4d8", "37b51d194a7513e45b56f6524f2d51f2", ), - ( - pytest.lazy_fixture("gs"), - "md5", - "acbd18db4cc2f85cedef654fccc4a4d8", - "37b51d194a7513e45b56f6524f2d51f2", - ), ( pytest.lazy_fixture("hdfs"), "checksum", diff --git a/tests/unit/output/test_gs.py b/tests/unit/output/test_gs.py deleted file mode 100644 index 6868a8f81..000000000 --- a/tests/unit/output/test_gs.py +++ /dev/null @@ -1,7 +0,0 @@ -from dvc.output.gs import GSOutput -from tests.unit.output.test_local import TestLocalOutput - - -class TestGSOutput(TestLocalOutput): - def _get_cls(self): - return GSOutput