From f42c054253e0b24bf9097c05e4d30c392c6b5401 Mon Sep 17 00:00:00 2001 From: Fariz Rahman Date: Fri, 17 Jul 2020 10:26:07 +0530 Subject: [PATCH 01/13] key api --- dvc/tree/s3.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/dvc/tree/s3.py b/dvc/tree/s3.py index 17d58301f1..a518e3d15a 100644 --- a/dvc/tree/s3.py +++ b/dvc/tree/s3.py @@ -54,19 +54,35 @@ def __init__(self, repo, config): self._append_aws_grants_to_extra_args(config) + self.key_id = config.get("s3_key_id") + self.key_secret = config.get("s3_key_secret") + shared_creds = config.get("credentialpath") if shared_creds: os.environ.setdefault("AWS_SHARED_CREDENTIALS_FILE", shared_creds) + self._validate_config() + + + def _validate_config(self): + if bool(self.key_id) != bool(self.key_secret): + raise DvcException("Provide either both or none of `s3_key_id` and `s3_key_secret`.") @wrap_prop(threading.Lock()) @cached_property def s3(self): import boto3 - - session = boto3.session.Session( - profile_name=self.profile, region_name=self.region + + session_opts = dict( + profile_name = self.profile, + region_name = self.region ) + if self.key_id and self.key_secret: + session_opts['aws_access_key_id'] = self.key_id + session_opts['aws_secret_access_key'] = self.key_secret + + session = boto3.session.Session(**session_opts) + return session.client( "s3", endpoint_url=self.endpoint_url, use_ssl=self.use_ssl ) From 63fd3be5ce6161293385869393e65d689b42af8c Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Fri, 17 Jul 2020 05:04:45 +0000 Subject: [PATCH 02/13] Restyled by black --- dvc/tree/s3.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/dvc/tree/s3.py b/dvc/tree/s3.py index a518e3d15a..a1a1a85024 100644 --- a/dvc/tree/s3.py +++ b/dvc/tree/s3.py @@ -62,24 +62,22 @@ def __init__(self, repo, config): os.environ.setdefault("AWS_SHARED_CREDENTIALS_FILE", shared_creds) self._validate_config() - def _validate_config(self): if bool(self.key_id) != bool(self.key_secret): - raise DvcException("Provide either both or none of `s3_key_id` and `s3_key_secret`.") + raise DvcException( + "Provide either both or none of `s3_key_id` and `s3_key_secret`." + ) @wrap_prop(threading.Lock()) @cached_property def s3(self): import boto3 - - session_opts = dict( - profile_name = self.profile, - region_name = self.region - ) + + session_opts = dict(profile_name=self.profile, region_name=self.region) if self.key_id and self.key_secret: - session_opts['aws_access_key_id'] = self.key_id - session_opts['aws_secret_access_key'] = self.key_secret + session_opts["aws_access_key_id"] = self.key_id + session_opts["aws_secret_access_key"] = self.key_secret session = boto3.session.Session(**session_opts) From 7b2bc4a58448686a2ae0705d5cf472ce46db3945 Mon Sep 17 00:00:00 2001 From: Fariz Rahman Date: Fri, 17 Jul 2020 11:06:21 +0530 Subject: [PATCH 03/13] test --- dvc/config.py | 2 ++ dvc/tree/s3.py | 9 ++------- tests/unit/remote/test_s3.py | 10 +++++++++- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/dvc/config.py b/dvc/config.py index a43cb16f2f..2b92124376 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -149,6 +149,8 @@ class RelPath(str): "profile": str, "credentialpath": str, "endpointurl": str, + Optional("s3_key_id", default=None), + Optional("s3_key_secret", default=None), Optional("listobjects", default=False): Bool, Optional("use_ssl", default=True): Bool, "sse": str, diff --git a/dvc/tree/s3.py b/dvc/tree/s3.py index a1a1a85024..f9a1fcc6a3 100644 --- a/dvc/tree/s3.py +++ b/dvc/tree/s3.py @@ -60,13 +60,7 @@ def __init__(self, repo, config): shared_creds = config.get("credentialpath") if shared_creds: os.environ.setdefault("AWS_SHARED_CREDENTIALS_FILE", shared_creds) - self._validate_config() - def _validate_config(self): - if bool(self.key_id) != bool(self.key_secret): - raise DvcException( - "Provide either both or none of `s3_key_id` and `s3_key_secret`." - ) @wrap_prop(threading.Lock()) @cached_property @@ -75,8 +69,9 @@ def s3(self): session_opts = dict(profile_name=self.profile, region_name=self.region) - if self.key_id and self.key_secret: + if self.key_id: session_opts["aws_access_key_id"] = self.key_id + if self.key_secret: session_opts["aws_secret_access_key"] = self.key_secret session = boto3.session.Session(**session_opts) diff --git a/tests/unit/remote/test_s3.py b/tests/unit/remote/test_s3.py index 570371c0c7..b32ff11d18 100644 --- a/tests/unit/remote/test_s3.py +++ b/tests/unit/remote/test_s3.py @@ -6,7 +6,8 @@ bucket_name = "bucket-name" prefix = "some/prefix" url = f"s3://{bucket_name}/{prefix}" - +key_id = "key_id" +key_secret = "key_secret" @pytest.fixture(autouse=True) def grants(): @@ -57,3 +58,10 @@ def test_grants_mutually_exclusive_acl_error(dvc, grants): def test_sse_kms_key_id(dvc): tree = S3RemoteTree(dvc, {"url": url, "sse_kms_key_id": "key"}) assert tree.extra_args["SSEKMSKeyId"] == "key" + + +def test_key_id_and_secret(dvc): + tree = S3RemoteTree(dvc, {"url": url, "s3_key_id"= key_id, "s3_key_secret"=key_secret}) + assert tree.key_id == key_id + assert tree.key_secret = key_secret + From 76e8b62d79889bda8fbe57e45fad54153b2e49e2 Mon Sep 17 00:00:00 2001 From: Fariz Rahman Date: Fri, 17 Jul 2020 11:10:13 +0530 Subject: [PATCH 04/13] fixes --- dvc/config.py | 4 ++-- tests/unit/remote/test_s3.py | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/dvc/config.py b/dvc/config.py index 2b92124376..3cc0cb5369 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -149,8 +149,8 @@ class RelPath(str): "profile": str, "credentialpath": str, "endpointurl": str, - Optional("s3_key_id", default=None), - Optional("s3_key_secret", default=None), + Optional("s3_key_id", default=None): str, + Optional("s3_key_secret", default=None): str, Optional("listobjects", default=False): Bool, Optional("use_ssl", default=True): Bool, "sse": str, diff --git a/tests/unit/remote/test_s3.py b/tests/unit/remote/test_s3.py index b32ff11d18..74c3346a10 100644 --- a/tests/unit/remote/test_s3.py +++ b/tests/unit/remote/test_s3.py @@ -61,7 +61,6 @@ def test_sse_kms_key_id(dvc): def test_key_id_and_secret(dvc): - tree = S3RemoteTree(dvc, {"url": url, "s3_key_id"= key_id, "s3_key_secret"=key_secret}) + tree = S3RemoteTree(dvc, {"url": url, "s3_key_id": key_id, "s3_key_secret": key_secret}) assert tree.key_id == key_id assert tree.key_secret = key_secret - From c23ead60e56ebe42c8be2040af29897eda1e6146 Mon Sep 17 00:00:00 2001 From: Fariz Rahman Date: Fri, 17 Jul 2020 11:11:42 +0530 Subject: [PATCH 05/13] test --- tests/unit/remote/test_s3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/remote/test_s3.py b/tests/unit/remote/test_s3.py index 74c3346a10..882bc51f94 100644 --- a/tests/unit/remote/test_s3.py +++ b/tests/unit/remote/test_s3.py @@ -63,4 +63,4 @@ def test_sse_kms_key_id(dvc): def test_key_id_and_secret(dvc): tree = S3RemoteTree(dvc, {"url": url, "s3_key_id": key_id, "s3_key_secret": key_secret}) assert tree.key_id == key_id - assert tree.key_secret = key_secret + assert tree.key_secret == key_secret From e9ce906588902594d81aeae3d2e138e7e1776abb Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Fri, 17 Jul 2020 05:42:04 +0000 Subject: [PATCH 06/13] Restyled by black --- dvc/tree/s3.py | 1 - tests/unit/remote/test_s3.py | 5 ++++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dvc/tree/s3.py b/dvc/tree/s3.py index f9a1fcc6a3..574a47d48b 100644 --- a/dvc/tree/s3.py +++ b/dvc/tree/s3.py @@ -61,7 +61,6 @@ def __init__(self, repo, config): if shared_creds: os.environ.setdefault("AWS_SHARED_CREDENTIALS_FILE", shared_creds) - @wrap_prop(threading.Lock()) @cached_property def s3(self): diff --git a/tests/unit/remote/test_s3.py b/tests/unit/remote/test_s3.py index 882bc51f94..4ecddeefec 100644 --- a/tests/unit/remote/test_s3.py +++ b/tests/unit/remote/test_s3.py @@ -9,6 +9,7 @@ key_id = "key_id" key_secret = "key_secret" + @pytest.fixture(autouse=True) def grants(): return { @@ -61,6 +62,8 @@ def test_sse_kms_key_id(dvc): def test_key_id_and_secret(dvc): - tree = S3RemoteTree(dvc, {"url": url, "s3_key_id": key_id, "s3_key_secret": key_secret}) + tree = S3RemoteTree( + dvc, {"url": url, "s3_key_id": key_id, "s3_key_secret": key_secret} + ) assert tree.key_id == key_id assert tree.key_secret == key_secret From 937cfabd8adc60dd91c82a64ebaeacfecb58a238 Mon Sep 17 00:00:00 2001 From: Fariz Rahman Date: Sat, 18 Jul 2020 06:12:25 +0530 Subject: [PATCH 07/13] Trigger CI From e8526257bf1afa0dc4ab3cc982f57c6fa2dd0d7a Mon Sep 17 00:00:00 2001 From: Fariz Rahman Date: Sat, 18 Jul 2020 05:39:07 +0400 Subject: [PATCH 08/13] default val --- dvc/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/config.py b/dvc/config.py index 3cc0cb5369..7bfa16d915 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -149,8 +149,8 @@ class RelPath(str): "profile": str, "credentialpath": str, "endpointurl": str, - Optional("s3_key_id", default=None): str, - Optional("s3_key_secret", default=None): str, + Optional("s3_key_id", default=""): str, + Optional("s3_key_secret", default=""): str, Optional("listobjects", default=False): Bool, Optional("use_ssl", default=True): Bool, "sse": str, From e800d534e806e4a2af877a003c823771c33b7fb7 Mon Sep 17 00:00:00 2001 From: Fariz Rahman Date: Sat, 18 Jul 2020 08:35:41 +0400 Subject: [PATCH 09/13] remove redundant default Co-authored-by: Ruslan Kuprieiev --- dvc/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/config.py b/dvc/config.py index 7bfa16d915..058a698970 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -149,8 +149,8 @@ class RelPath(str): "profile": str, "credentialpath": str, "endpointurl": str, - Optional("s3_key_id", default=""): str, - Optional("s3_key_secret", default=""): str, + "s3_key_id": str, + "s3_key_secret": str, Optional("listobjects", default=False): Bool, Optional("use_ssl", default=True): Bool, "sse": str, From 872fd3d0199454d48eee5b5808f1087904782b7b Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sat, 18 Jul 2020 07:42:41 +0300 Subject: [PATCH 10/13] Update tests/unit/remote/test_s3.py --- tests/unit/remote/test_s3.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/remote/test_s3.py b/tests/unit/remote/test_s3.py index 4ecddeefec..3801fb3d65 100644 --- a/tests/unit/remote/test_s3.py +++ b/tests/unit/remote/test_s3.py @@ -6,8 +6,8 @@ bucket_name = "bucket-name" prefix = "some/prefix" url = f"s3://{bucket_name}/{prefix}" -key_id = "key_id" -key_secret = "key_secret" +key_id = "key-id" +key_secret = "key-secret" @pytest.fixture(autouse=True) From d7aebc51a1cb3a129f03c97f797c8e4c63345170 Mon Sep 17 00:00:00 2001 From: Fariz Rahman Date: Sat, 18 Jul 2020 11:13:43 +0530 Subject: [PATCH 11/13] rename fields --- dvc/config.py | 4 ++-- dvc/tree/s3.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/dvc/config.py b/dvc/config.py index 3cc0cb5369..2cd9afe5b7 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -149,8 +149,8 @@ class RelPath(str): "profile": str, "credentialpath": str, "endpointurl": str, - Optional("s3_key_id", default=None): str, - Optional("s3_key_secret", default=None): str, + Optional("access_key_id", default=""): str, + Optional("secret_access_key", default=""): str, Optional("listobjects", default=False): Bool, Optional("use_ssl", default=True): Bool, "sse": str, diff --git a/dvc/tree/s3.py b/dvc/tree/s3.py index 574a47d48b..37c900ff2b 100644 --- a/dvc/tree/s3.py +++ b/dvc/tree/s3.py @@ -54,8 +54,8 @@ def __init__(self, repo, config): self._append_aws_grants_to_extra_args(config) - self.key_id = config.get("s3_key_id") - self.key_secret = config.get("s3_key_secret") + self.access_key_id = config.get("access_key_id") + self.secret_access_key = config.get("secret_access_key") shared_creds = config.get("credentialpath") if shared_creds: @@ -68,10 +68,10 @@ def s3(self): session_opts = dict(profile_name=self.profile, region_name=self.region) - if self.key_id: - session_opts["aws_access_key_id"] = self.key_id - if self.key_secret: - session_opts["aws_secret_access_key"] = self.key_secret + if self.acess_key_id: + session_opts["aws_access_key_id"] = self.access_key_id + if self.secret_access_key: + session_opts["aws_secret_access_key"] = self.secret_access_key session = boto3.session.Session(**session_opts) From 915d27d7595190fd5c1acf2130d066b385436e75 Mon Sep 17 00:00:00 2001 From: Fariz Rahman Date: Sat, 18 Jul 2020 11:19:04 +0530 Subject: [PATCH 12/13] update test --- tests/unit/remote/test_s3.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/unit/remote/test_s3.py b/tests/unit/remote/test_s3.py index 4ecddeefec..aef97a82cf 100644 --- a/tests/unit/remote/test_s3.py +++ b/tests/unit/remote/test_s3.py @@ -63,7 +63,8 @@ def test_sse_kms_key_id(dvc): def test_key_id_and_secret(dvc): tree = S3RemoteTree( - dvc, {"url": url, "s3_key_id": key_id, "s3_key_secret": key_secret} + dvc, + {"url": url, "access_key_id": key_id, "secret_access_key": key_secret}, ) - assert tree.key_id == key_id - assert tree.key_secret == key_secret + assert tree.access_key_id == key_id + assert tree.secret_access_key == key_secret From 8d289eb31011c23c5f7cbd963b3b4249d3524821 Mon Sep 17 00:00:00 2001 From: Fariz Rahman Date: Sat, 18 Jul 2020 11:38:00 +0530 Subject: [PATCH 13/13] key_id->secret_key_id --- dvc/tree/s3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/tree/s3.py b/dvc/tree/s3.py index 37c900ff2b..154c757992 100644 --- a/dvc/tree/s3.py +++ b/dvc/tree/s3.py @@ -68,7 +68,7 @@ def s3(self): session_opts = dict(profile_name=self.profile, region_name=self.region) - if self.acess_key_id: + if self.access_key_id: session_opts["aws_access_key_id"] = self.access_key_id if self.secret_access_key: session_opts["aws_secret_access_key"] = self.secret_access_key