From d132aada033acc6bfeb29236ada441453d4d47cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Debon?= Date: Thu, 5 Dec 2019 10:26:22 +0100 Subject: [PATCH 1/2] Create update_permissions_field function --- substra/sdk/client.py | 47 +++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/substra/sdk/client.py b/substra/sdk/client.py index a385322b..182d91fa 100644 --- a/substra/sdk/client.py +++ b/substra/sdk/client.py @@ -110,6 +110,20 @@ def set_user(self): }) self.client.set_config(self._current_profile, self._profile_name) + def update_permissions_field(self, data, permissions_field='permissions'): + if permissions_field: + if permissions_field not in data: + default_permissions = { + 'public': False, + 'authorized_ids': [], + } + data[permissions_field] = default_permissions + # XXX workaround because backend accepts only Form Data body. This is due to + # the fact that backend expects both file objects and payload in the + # same request + data = _flatten_permissions(data, field_name=permissions_field) + return data + def add_profile(self, profile_name, username, password, url, version='0.0', insecure=False): """Add new profile (in-memory only).""" profile = cfg.create_profile( @@ -121,24 +135,10 @@ def add_profile(self, profile_name, username, password, url, version='0.0', inse keyring.set_password(profile_name, username, password) return self._set_current_profile(profile_name, profile) - def _add(self, asset, data, files=None, timeout=False, exist_ok=False, - json_encoding=False, permissions_field='permissions'): + def _add(self, asset, data, files=None, timeout=False, exist_ok=False, json_encoding=False): """Add asset.""" data = deepcopy(data) # make a deep copy for avoiding modification by reference files = files or {} - - if permissions_field: - if permissions_field not in data: - default_permissions = { - 'public': False, - 'authorized_ids': [], - } - data[permissions_field] = default_permissions - # XXX workaround because backend accepts only Form Data body. This is due to - # the fact that backend expects both file objects and payload in the - # same request - data = _flatten_permissions(data, field_name=permissions_field) - requests_kwargs = {} if files: requests_kwargs['files'] = files @@ -192,6 +192,7 @@ def add_data_sample(self, data, local=True, timeout=False, exist_ok=False): existing asset will be returned. """ + data = self.update_permissions_field(data) if 'paths' in data: raise ValueError("data: invalid 'paths' field") if 'path' not in data: @@ -234,6 +235,7 @@ def add_data_samples(self, data, local=True, timeout=False): large amount of data it is recommended to add them one by one. It allows a better control in case of failures. """ + data = self.update_permissions_field(data) if 'path' in data: raise ValueError("data: invalid 'path' field") if 'paths' not in data: @@ -263,6 +265,7 @@ def add_dataset(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ + data = self.update_permissions_field(data) attributes = ['data_opener', 'description'] with utils.extract_files(data, attributes) as (data, files): res = self._add( @@ -296,6 +299,7 @@ def add_objective(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ + data = self.update_permissions_field(data) attributes = ['metrics', 'description'] with utils.extract_files(data, attributes) as (data, files): res = self._add( @@ -326,6 +330,7 @@ def add_algo(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ + data = self.update_permissions_field(data) attributes = ['file', 'description'] with utils.extract_files(data, attributes) as (data, files): res = self._add( @@ -353,6 +358,7 @@ def add_aggregate_algo(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ + data = self.update_permissions_field(data) attributes = ['file', 'description'] with utils.extract_files(data, attributes) as (data, files): res = self._add( @@ -380,6 +386,7 @@ def add_composite_algo(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ + data = self.update_permissions_field(data) attributes = ['file', 'description'] with utils.extract_files(data, attributes) as (data, files): res = self._add( @@ -410,8 +417,7 @@ def add_traintuple(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ - res = self._add(assets.TRAINTUPLE, data, timeout=timeout, - exist_ok=exist_ok, permissions_field=None) + res = self._add(assets.TRAINTUPLE, data, timeout=timeout, exist_ok=exist_ok) # The backend has inconsistent API responses when getting or adding an asset (with much # less data when responding to adds). A second GET request hides the discrepancies. @@ -461,9 +467,7 @@ def add_composite_traintuple(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ - res = self._add(assets.COMPOSITE_TRAINTUPLE, data, timeout=timeout, - exist_ok=exist_ok, - permissions_field='out_model_trunk_permissions') + res = self._add(assets.COMPOSITE_TRAINTUPLE, data, timeout=timeout, exist_ok=exist_ok) # The backend has inconsistent API responses when getting or adding an asset (with much # less data when responding to adds). A second GET request hides the discrepancies. @@ -487,8 +491,7 @@ def add_testtuple(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ - res = self._add(assets.TESTTUPLE, data, timeout=timeout, - exist_ok=exist_ok) + res = self._add(assets.TESTTUPLE, data, timeout=timeout, exist_ok=exist_ok) # The backend has inconsistent API responses when getting or adding an asset (with much # less data when responding to adds). A second GET request hides the discrepancies. From 6dd5925aacd4b1cc9b888f2058f7ec75c9715c03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Debon?= Date: Thu, 5 Dec 2019 12:05:43 +0100 Subject: [PATCH 2/2] Make update_permissions_field private --- substra/sdk/client.py | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/substra/sdk/client.py b/substra/sdk/client.py index 182d91fa..72a51148 100644 --- a/substra/sdk/client.py +++ b/substra/sdk/client.py @@ -40,6 +40,20 @@ def _flatten_permissions(data, field_name): return data +def _update_permissions_field(data, permissions_field='permissions'): + if permissions_field not in data: + default_permissions = { + 'public': False, + 'authorized_ids': [], + } + data[permissions_field] = default_permissions + # XXX workaround because backend accepts only Form Data body. This is due to + # the fact that backend expects both file objects and payload in the + # same request + data = _flatten_permissions(data, field_name=permissions_field) + return data + + class Client(object): def __init__(self, config_path=None, profile_name=None, user_path=None): @@ -110,20 +124,6 @@ def set_user(self): }) self.client.set_config(self._current_profile, self._profile_name) - def update_permissions_field(self, data, permissions_field='permissions'): - if permissions_field: - if permissions_field not in data: - default_permissions = { - 'public': False, - 'authorized_ids': [], - } - data[permissions_field] = default_permissions - # XXX workaround because backend accepts only Form Data body. This is due to - # the fact that backend expects both file objects and payload in the - # same request - data = _flatten_permissions(data, field_name=permissions_field) - return data - def add_profile(self, profile_name, username, password, url, version='0.0', insecure=False): """Add new profile (in-memory only).""" profile = cfg.create_profile( @@ -192,7 +192,7 @@ def add_data_sample(self, data, local=True, timeout=False, exist_ok=False): existing asset will be returned. """ - data = self.update_permissions_field(data) + data = _update_permissions_field(data) if 'paths' in data: raise ValueError("data: invalid 'paths' field") if 'path' not in data: @@ -235,7 +235,7 @@ def add_data_samples(self, data, local=True, timeout=False): large amount of data it is recommended to add them one by one. It allows a better control in case of failures. """ - data = self.update_permissions_field(data) + data = _update_permissions_field(data) if 'path' in data: raise ValueError("data: invalid 'path' field") if 'paths' not in data: @@ -265,7 +265,7 @@ def add_dataset(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ - data = self.update_permissions_field(data) + data = _update_permissions_field(data) attributes = ['data_opener', 'description'] with utils.extract_files(data, attributes) as (data, files): res = self._add( @@ -299,7 +299,7 @@ def add_objective(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ - data = self.update_permissions_field(data) + data = _update_permissions_field(data) attributes = ['metrics', 'description'] with utils.extract_files(data, attributes) as (data, files): res = self._add( @@ -330,7 +330,7 @@ def add_algo(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ - data = self.update_permissions_field(data) + data = _update_permissions_field(data) attributes = ['file', 'description'] with utils.extract_files(data, attributes) as (data, files): res = self._add( @@ -358,7 +358,7 @@ def add_aggregate_algo(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ - data = self.update_permissions_field(data) + data = _update_permissions_field(data) attributes = ['file', 'description'] with utils.extract_files(data, attributes) as (data, files): res = self._add( @@ -386,7 +386,7 @@ def add_composite_algo(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ - data = self.update_permissions_field(data) + data = _update_permissions_field(data) attributes = ['file', 'description'] with utils.extract_files(data, attributes) as (data, files): res = self._add( @@ -467,6 +467,7 @@ def add_composite_traintuple(self, data, timeout=False, exist_ok=False): If `exist_ok` is true, `AlreadyExists` exceptions will be ignored and the existing asset will be returned. """ + data = _update_permissions_field(data, permissions_field='out_model_trunk_permissions') res = self._add(assets.COMPOSITE_TRAINTUPLE, data, timeout=timeout, exist_ok=exist_ok) # The backend has inconsistent API responses when getting or adding an asset (with much