From f06d6ac83b6810f9dc9c64c3b6a9c0eeaf9635a8 Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Fri, 21 Jan 2022 18:24:43 +0000 Subject: [PATCH 01/49] Add initial unittests for the oauth flow --- .gitignore | 4 + .../test => tests}/file_list_test.txt | 0 tests/test_authentication.py | 73 ++++++++++++++ tests/test_oauth.py | 98 +++++++++++++++++++ 4 files changed, 175 insertions(+) create mode 100644 .gitignore rename {nlds_client/test => tests}/file_list_test.txt (100%) create mode 100644 tests/test_authentication.py create mode 100644 tests/test_oauth.py diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..798a11a --- /dev/null +++ b/.gitignore @@ -0,0 +1,4 @@ +**/__pycache__/ +.pytest_cache/ +*egg-info +.venv/ \ No newline at end of file diff --git a/nlds_client/test/file_list_test.txt b/tests/file_list_test.txt similarity index 100% rename from nlds_client/test/file_list_test.txt rename to tests/file_list_test.txt diff --git a/tests/test_authentication.py b/tests/test_authentication.py new file mode 100644 index 0000000..8036991 --- /dev/null +++ b/tests/test_authentication.py @@ -0,0 +1,73 @@ +import os +import unittest +from unittest.mock import patch +import nlds_client.clientlib.authentication as clau +import json +import requests +import nlds_client.clientlib.exceptions as exc + +TEST_USERNAME = 'testuser' +TEST_PASSWORD = 'testpswd' +TEST_TOKEN_TEXT = '{"access_token": "pojp", "expires_in": 36000, "token_type": "Bearer", "scope": "test scope", "refresh_token": "popojp"}' + +class MockResponse: + def __init__(self, status_code, text=TEST_TOKEN_TEXT): + self.status_code = status_code + self.text = text + + +def mock_requests_post(*args, **kwargs): + if kwargs['data']['username'] == TEST_USERNAME and kwargs['data']['password'] == TEST_PASSWORD: + return MockResponse(requests.codes.ok) + elif kwargs['data']['username'] == TEST_USERNAME and kwargs['data']['password'] != TEST_PASSWORD: + return MockResponse(requests.codes.unauthorized) + elif kwargs['data']['username'] != TEST_USERNAME: + return MockResponse(requests.codes.forbidden) + + +class TestAuthentication(unittest.TestCase): + def setUp(self) -> None: + template_filename = os.path.join(os.path.dirname(__file__), '../templates/nlds-config.j2') + fh = open(template_filename) + self.config = json.load(fh) + + def test_response_ok(self): + response = MockResponse(requests.codes.ok) + self.assertEqual(clau.process_fetch_oauth2_token_response(self.config, response), + response) + + def test_response_bad_request(self): + response = MockResponse(requests.codes.bad_request) + with self.assertRaises(exc.RequestError): + clau.process_fetch_oauth2_token_response(self.config, response) + + def test_response_unauthorised(self): + response = MockResponse(requests.codes.unauthorized) + with self.assertRaises(exc.AuthenticationError): + clau.process_fetch_oauth2_token_response(self.config, response) + + def test_response_forbidden(self): + response = MockResponse(requests.codes.forbidden) + with self.assertRaises(exc.AuthenticationError): + clau.process_fetch_oauth2_token_response(self.config, response) + + def test_response_not_found(self): + response = MockResponse(requests.codes.not_found) + with self.assertRaises(exc.RequestError): + clau.process_fetch_oauth2_token_response(self.config, response) + + def test_response_undefined(self): + response = MockResponse(None) + with self.assertRaises(exc.RequestError): + clau.process_fetch_oauth2_token_response(self.config, response) + + @patch('requests.post', side_effect=mock_requests_post) + def test_fetch(self, mock_post): + self.assertEqual(clau.fetch_oauth2_token(self.config, TEST_USERNAME, TEST_PASSWORD), json.loads(TEST_TOKEN_TEXT)) + with self.assertRaises(exc.AuthenticationError): + clau.fetch_oauth2_token(self.config, TEST_USERNAME, '') + with self.assertRaises(exc.AuthenticationError): + clau.fetch_oauth2_token(self.config, '', TEST_PASSWORD) + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_oauth.py b/tests/test_oauth.py new file mode 100644 index 0000000..d9aa947 --- /dev/null +++ b/tests/test_oauth.py @@ -0,0 +1,98 @@ +import os +import pytest +import requests +import json +import nlds_client.clientlib.authentication as clau +import nlds_client.clientlib.config as clcnf +import nlds_client.clientlib.exceptions as exc +import dotenv +import copy + +# Load environment files from .env file +dotenv.load_dotenv() + +TEST_USERNAME = os.environ['USERNAME'] +TEST_PASSWORD = os.environ['PASSWORD'] +TEST_TOKEN_TEXT = os.environ['TOKEN_TEXT'] +TEST_CONFIG_FILE = os.environ.get('CONFIG_FILE') +EDGE_VALUES = ['', None, ' ',] + +@pytest.fixture +def basic_config(): + # Read the template config file into a json object + config_filename = os.path.join(os.path.dirname(__file__), '../nlds_client/templates/nlds-config.j2') + fh = open(config_filename) + return json.load(fh) + +@pytest.fixture +def functional_config(): + # Read the users actual config file so test calls can be made to the API + if TEST_CONFIG_FILE is None: + return clcnf.load_config() + else: + fh = open(TEST_CONFIG_FILE) + return json.load(fh) + +class MockResponse: + # A mock response object for checking error handling capabilities + def __init__(self, status_code=200, text=TEST_TOKEN_TEXT): + self.status_code = status_code + self.text = text + +def test_get_username_password(monkeypatch, basic_config): + # Mock the username and password getting by overriding the input and getpass functions + monkeypatch.setattr('builtins.input', lambda _: TEST_USERNAME) + monkeypatch.setattr('getpass.getpass', lambda _: TEST_PASSWORD) + + userpswd_output = clau.get_username_password(basic_config) + + assert len(userpswd_output) == 2 + assert userpswd_output[0] == TEST_USERNAME + assert userpswd_output[1] == TEST_PASSWORD + +@pytest.mark.parametrize("status_code", [ + requests.codes.bad_request, + requests.codes.unauthorized, + requests.codes.forbidden, + requests.codes.not_found, + None, +]) +def test_process_fetch_oauth2_token_response(basic_config, status_code): + clau.process_fetch_oauth2_token_response(basic_config, MockResponse(status_code=200)) + with pytest.raises(exc.StatusCodeError): + clau.process_fetch_oauth2_token_response(basic_config, MockResponse(status_code=status_code)) + + +@pytest.mark.parametrize("test_value", EDGE_VALUES) +def test_fetch_oauth_token(functional_config, test_value): + # Test correct username and password + token = clau.fetch_oauth2_token(functional_config, TEST_USERNAME, TEST_PASSWORD) + assert isinstance(token, dict) + + # Test incorrect password raises exception + with pytest.raises(exc.RequestError): + clau.fetch_oauth2_token(functional_config, TEST_USERNAME, 'incorrect_password') + clau.fetch_oauth2_token(functional_config, TEST_USERNAME, test_value) + # Test incorrect/invalid user raises exception + with pytest.raises(exc.RequestError): + clau.fetch_oauth2_token(functional_config, 'invalid_user', TEST_PASSWORD) + clau.fetch_oauth2_token(functional_config, test_value, TEST_PASSWORD) + # Test incorrect user & password raises exception. + with pytest.raises(exc.StatusCodeError): + clau.fetch_oauth2_token(functional_config, test_value, test_value) + +@pytest.mark.parametrize("test_value", EDGE_VALUES) +def test_fetch_oauth_config(monkeypatch, functional_config, test_value): + monkeypatch.setattr('save_token', None) + + modified_config = copy.deepcopy(functional_config) + modified_config['authentication']['oauth_client_id'] = test_value + with pytest.raises(Exception): + clau.fetch_oauth2_token(functional_config, TEST_USERNAME, TEST_PASSWORD) + + + modified_config = copy.deepcopy(functional_config) + modified_config['authentication']['oauth_client_secret'] = test_value + with pytest.raises(Exception): + clau.fetch_oauth2_token(functional_config, TEST_USERNAME, TEST_PASSWORD) + From d0101867380d988e2b6def9050f04a9de93475a3 Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Fri, 21 Jan 2022 18:25:14 +0000 Subject: [PATCH 02/49] Fix redirect bug caused by missing slash in url --- nlds_client/clientlib/transactions.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 1b3110c..93a89d1 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -1,8 +1,10 @@ +from json.decoder import JSONDecodeError import requests import json import uuid import urllib.parse import os +import traceback from typing import List, Dict @@ -28,7 +30,7 @@ def construct_server_url(config: Dict): url = urllib.parse.urljoin(config["server"]["url"], "/".join([config["server"]["api"], "files"]) - ) + ) + "/" return url @@ -246,7 +248,7 @@ def get_filelist(filelist: List[str]=[], user = get_user(config, user) group = get_group(config, group) transaction_id = uuid.uuid4() - url = construct_server_url(config) + "/getlist" + url = construct_server_url(config) + "getlist" MAX_LOOPS = 2 while c_try < MAX_LOOPS: From 076210d030930bfbc14a702a3f91f4bcf55ba60f Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Fri, 21 Jan 2022 18:26:04 +0000 Subject: [PATCH 03/49] Ignore directories created to test indexing --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 798a11a..8932e95 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ **/__pycache__/ .pytest_cache/ *egg-info -.venv/ \ No newline at end of file +.venv/ +tests/test_dir* \ No newline at end of file From ef9b93d403a8af7383ad0722e213ef948a15167c Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Mon, 24 Jan 2022 16:40:53 +0000 Subject: [PATCH 04/49] Add option to ignore certificate verification An optional section 'options' is added to the config file which can be used to set global options within the client - the first of which implemented is the boolean flag able to be set to ignore certificate verification within the http request. This is to allow interaction with the staging server which has a self-signed ssl certificate. --- nlds_client/clientlib/config.py | 20 +++++++++++++++++++- nlds_client/clientlib/transactions.py | 15 +++++++++------ nlds_client/templates/nlds-config.j2 | 3 +++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/nlds_client/clientlib/config.py b/nlds_client/clientlib/config.py index 91828e6..2b9376f 100644 --- a/nlds_client/clientlib/config.py +++ b/nlds_client/clientlib/config.py @@ -1,5 +1,7 @@ import json import os.path + +from click import option from nlds_client.clientlib.nlds_client_setup import CONFIG_FILE_LOCATION from nlds_client.clientlib.exceptions import ConfigError @@ -45,7 +47,6 @@ def validate_config_file(json_config): f"{key} in ['authentication'] section." ) - def load_config(): """Config file for the client contains: authentication : { @@ -98,3 +99,20 @@ def get_group(config, group): "default_group" in config["user"]): group = config["user"]["default_group"] return group + +_DEFAULT_OPTIONS = { + "verify_certificates": True +} +def get_option(config, option_name, section_name='option'): + """Get an option from either the config or the DEFAULT_OPTIONS dict.""" + if (section_name in config and + # Get value from config if option section and option present + option_name in config[section_name]): + option_value = config[section_name][option_name] + elif option_name in _DEFAULT_OPTIONS: + # Otherwise get the default value + option_value = _DEFAULT_OPTIONS[option_name] + else: + # Silently fail if option not specified in _DEFUALT_OPTIONS + option_value = None + return option_value \ No newline at end of file diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 93a89d1..8874a39 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -1,14 +1,13 @@ -from json.decoder import JSONDecodeError import requests import json import uuid import urllib.parse import os -import traceback from typing import List, Dict -from nlds_client.clientlib.config import load_config, get_user, get_group +from nlds_client.clientlib.config import load_config, get_user, get_group, \ + get_option from nlds_client.clientlib.authentication import load_token,\ get_username_password,\ fetch_oauth2_token,\ @@ -179,7 +178,8 @@ def get_file(filepath: str, user: str=None, group: str=None): response = requests.get( url, headers = token_headers, - params = input_params + params = input_params, + verify = get_option(config, 'verify_certificates') ) except requests.exceptions.ConnectionError: raise ConnectionError( @@ -288,7 +288,8 @@ def get_filelist(filelist: List[str]=[], url, headers = token_headers, params = input_params, - json = body_params + json = body_params, + verify = get_option(config, 'verify_certificates') ) except requests.exceptions.ConnectionError: raise ConnectionError( @@ -394,6 +395,7 @@ def put_file(filepath: str, user: str=None, group: str=None): url, headers = token_headers, params = input_params, + verify = get_option(config, 'verify_certificates') ) except requests.exceptions.ConnectionError: raise ConnectionError( @@ -499,7 +501,8 @@ def put_filelist(filelist: List[str]=[], url, headers = token_headers, params = input_params, - json = body_params + json = body_params, + verify = get_option(config, 'verify_certificates') ) except requests.exceptions.ConnectionError: raise ConnectionError( diff --git a/nlds_client/templates/nlds-config.j2 b/nlds_client/templates/nlds-config.j2 index 554b424..209736a 100644 --- a/nlds_client/templates/nlds-config.j2 +++ b/nlds_client/templates/nlds-config.j2 @@ -14,4 +14,7 @@ "oauth_scopes" : "{{ oauth_scopes }}", "oauth_token_file_location" : "~/.nlds-token" } + "options" : { + "verify_certificates": true + } } From 5015e4198a9a13adcb7d0b13486b621edd7c88b0 Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Tue, 15 Feb 2022 16:44:35 +0000 Subject: [PATCH 05/49] Add config option to fully resolve filepaths --- nlds_client/clientlib/config.py | 3 ++- nlds_client/clientlib/transactions.py | 23 ++++++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/nlds_client/clientlib/config.py b/nlds_client/clientlib/config.py index 2b9376f..319dc93 100644 --- a/nlds_client/clientlib/config.py +++ b/nlds_client/clientlib/config.py @@ -101,7 +101,8 @@ def get_group(config, group): return group _DEFAULT_OPTIONS = { - "verify_certificates": True + "verify_certificates": True, + "resolve_filenames": True } def get_option(config, option_name, section_name='option'): """Get an option from either the config or the DEFAULT_OPTIONS dict.""" diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 8874a39..dd88d2e 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -3,6 +3,7 @@ import uuid import urllib.parse import os +import pathlib from typing import List, Dict @@ -143,6 +144,11 @@ def get_file(filepath: str, user: str=None, group: str=None): url = construct_server_url(config) MAX_LOOPS = 2 + # Resolve path to file (i.e. make absolute) if configured so + if get_option(config, "resolve_filenames"): + # Convert to a pathlib.Path and then back to a string + filepath = str(pathlib.Path(filepath).resolve()) + while c_try < MAX_LOOPS: # get an OAuth token if we fail then the file doesn't exist. @@ -251,6 +257,11 @@ def get_filelist(filelist: List[str]=[], url = construct_server_url(config) + "getlist" MAX_LOOPS = 2 + # Resolve path to file (i.e. make absolute) if configured so + if get_option(config, "resolve_filenames"): + # Convert to a pathlib.Path and then back to a string + filelist = [str(pathlib.Path(fp).resolve()) for fp in filelist] + while c_try < MAX_LOOPS: # get an OAuth token if we fail then the file doesn't exist. @@ -329,7 +340,7 @@ def get_filelist(filelist: List[str]=[], return response_dict -def put_file(filepath: str, user: str=None, group: str=None): +def put_file(filepath: str, user: str=None, group: str=None, ignore_certificates: bool=None): """Make a request to put a single file into the NLDS. :param filepath: the path of the file to put into the storage @@ -356,6 +367,11 @@ def put_file(filepath: str, user: str=None, group: str=None): transaction_id = uuid.uuid4() url = construct_server_url(config) + # Resolve path to file (i.e. make absolute) if configured so + if get_option(config, "resolve_filenames"): + # Convert to a pathlib.Path and then back to a string + filepath = str(pathlib.Path(filepath).resolve()) + MAX_LOOPS = 2 while c_try < MAX_LOOPS: @@ -462,6 +478,11 @@ def put_filelist(filelist: List[str]=[], transaction_id = uuid.uuid4() url = construct_server_url(config) MAX_LOOPS = 2 + + # Resolve path to file (i.e. make absolute) if configured so + if get_option(config, "resolve_filenames"): + # Convert to a pathlib.Path and then back to a string + filelist = [str(pathlib.Path(fp).resolve()) for fp in filelist] while c_try < MAX_LOOPS: From ae317d9d85221f387318de9c49be1767642ea775 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Mon, 21 Feb 2022 17:12:17 +0000 Subject: [PATCH 06/49] Added information needed to access object storage from client - acces_key, etc. --- nlds_client/clientlib/config.py | 97 ++++++++++++++++++++++++--- nlds_client/clientlib/transactions.py | 58 +++++++++++++--- nlds_client/templates/nlds-config.j2 | 12 +++- 3 files changed, 146 insertions(+), 21 deletions(-) diff --git a/nlds_client/clientlib/config.py b/nlds_client/clientlib/config.py index 319dc93..c683753 100644 --- a/nlds_client/clientlib/config.py +++ b/nlds_client/clientlib/config.py @@ -18,7 +18,7 @@ def validate_config_file(json_config): for key in ["url", "api"]: try: - value = server_section[key] + _ = server_section[key] except KeyError: raise ConfigError( f"The config file at {CONFIG_FILE_LOCATION} does not contain " @@ -40,20 +40,73 @@ def validate_config_file(json_config): "oauth_scopes", "oauth_token_file_location"]: try: - value = auth_section[key] + _ = auth_section[key] except KeyError: raise ConfigError( f"The config file at {CONFIG_FILE_LOCATION} does not contain " f"{key} in ['authentication'] section." ) + # user section + try: + user_section = json_config["user"] + except KeyError: + raise ConfigError( + f"The config file at {CONFIG_FILE_LOCATION} does not contain an " + "['user'] section." + ) + + for key in ["default_user", + "default_group"]: + try: + _ = user_section[key] + except KeyError: + raise ConfigError( + f"The config file at {CONFIG_FILE_LOCATION} does not contain " + f"{key} in ['user'] section." + ) + + # object_storage section + try: + os_section = json_config["object_storage"] + except KeyError: + raise ConfigError( + f"The config file at {CONFIG_FILE_LOCATION} does not contain an " + "['object_storage'] section." + ) + + for key in ["access_key", # "tenancy" is optional and will default + "secret_key"]: # on the server + try: + _ = os_section[key] + except KeyError: + raise ConfigError( + f"The config file at {CONFIG_FILE_LOCATION} does not contain " + f"{key} in ['object_storage'] section." + ) + def load_config(): """Config file for the client contains: + server : { + url : , + api : + }, + user : { + default_user : , + default_group : + }, authentication : { - oauth_client_id : , - oauth_client_secret : , - oauth_token_url : , - oauth_token_introspect_url : + oauth_client_id : , + oauth_client_secret : , + oauth_token_url : , + oauth_scopes : , + oauth_token_file_location : + }, + object_storage : { + tenancy : (optional), + access_key : , + secret_key : + } """ # Location of config file is {CONFIG_FILE_LOCATION}. Open it, checking @@ -100,6 +153,7 @@ def get_group(config, group): group = config["user"]["default_group"] return group + _DEFAULT_OPTIONS = { "verify_certificates": True, "resolve_filenames": True @@ -114,6 +168,33 @@ def get_option(config, option_name, section_name='option'): # Otherwise get the default value option_value = _DEFAULT_OPTIONS[option_name] else: - # Silently fail if option not specified in _DEFUALT_OPTIONS + # Silently fail if option not specified in _DEFAULT_OPTIONS option_value = None - return option_value \ No newline at end of file + return option_value + + +def get_tenancy(config): + """Get the object storage tenancy from the config file. This is optional + so could be None.""" + tenancy = None + if ("object_storage" in config and + "tenancy" in config["object_storage"]): + tenancy = config["object_storage"]["tenancy"] + return tenancy + + +def get_access_key(config): + """Get the object storage access key from the config file. """ + access_key = "" + if ("object_storage" in config and + "access_key" in config["object_storage"]): + access_key = config["object_storage"]["access_key"] + return access_key + +def get_secret_key(config): + """Get the object storage secret key from the config file. """ + secret_key = "" + if ("object_storage" in config and + "secret_key" in config["object_storage"]): + secret_key = config["object_storage"]["secret_key"] + return secret_key \ No newline at end of file diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index dd88d2e..a1cc219 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -8,7 +8,8 @@ from typing import List, Dict from nlds_client.clientlib.config import load_config, get_user, get_group, \ - get_option + get_option, \ + get_tenancy, get_access_key, get_secret_key from nlds_client.clientlib.authentication import load_token,\ get_username_password,\ fetch_oauth2_token,\ @@ -140,6 +141,9 @@ def get_file(filepath: str, user: str=None, group: str=None): config = load_config() user = get_user(config, user) group = get_group(config, group) + tenancy = get_tenancy(config) + access_key = get_access_key(config) + secret_key = get_secret_key(config) transaction_id = uuid.uuid4() url = construct_server_url(config) MAX_LOOPS = 2 @@ -174,9 +178,15 @@ def get_file(filepath: str, user: str=None, group: str=None): # transaction_id: UUID # user: str # group: str + # access_key : str + # secret_key : str + # tenancy : str (optional) input_params = {"transaction_id" : transaction_id, "user" : user, "group" : group, + "access_key" : access_key, + "secret_key" : secret_key, + "tenancy" : tenancy, "filepath" : filepath} # make the request @@ -225,7 +235,7 @@ def get_file(filepath: str, user: str=None, group: str=None): return response_dict def get_filelist(filelist: List[str]=[], - user: str=None, group: str=None): + user: str=None, group: str=None): """Make a request to get a list of files from the NLDS. :param filelist: the list of filepaths to get from the storage :type filelist: List[string] @@ -253,6 +263,9 @@ def get_filelist(filelist: List[str]=[], config = load_config() user = get_user(config, user) group = get_group(config, group) + tenancy = get_tenancy(config) + access_key = get_access_key(config) + secret_key = get_secret_key(config) transaction_id = uuid.uuid4() url = construct_server_url(config) + "getlist" MAX_LOOPS = 2 @@ -287,10 +300,17 @@ def get_filelist(filelist: List[str]=[], # transaction_id: UUID # user: str # group: str + # access_key : str + # secret_key : str + # tenancy : str (optional) # and the filelist in the body input_params = {"transaction_id" : transaction_id, "user" : user, - "group" : group} + "group" : group, + "access_key" : access_key, + "secret_key" : secret_key, + "tenancy" : tenancy + } body_params = {"filelist" : filelist} # make the request @@ -340,7 +360,8 @@ def get_filelist(filelist: List[str]=[], return response_dict -def put_file(filepath: str, user: str=None, group: str=None, ignore_certificates: bool=None): +def put_file(filepath: str, user: str=None, group: str=None, + ignore_certificates: bool=None): """Make a request to put a single file into the NLDS. :param filepath: the path of the file to put into the storage @@ -364,6 +385,9 @@ def put_file(filepath: str, user: str=None, group: str=None, ignore_certificates config = load_config() user = get_user(config, user) group = get_group(config, group) + tenancy = get_tenancy(config) + access_key = get_access_key(config) + secret_key = get_secret_key(config) transaction_id = uuid.uuid4() url = construct_server_url(config) @@ -394,15 +418,20 @@ def put_file(filepath: str, user: str=None, group: str=None, ignore_certificates "Authorization" : f"Bearer {auth_token['access_token']}" } - # build the parameters. files/put requires: # transaction_id: UUID - # user: str - # group: str - # filepath : str (optional) + # user : str + # group : str + # access_key : str + # secret_key : str + # tenancy : str (optional) + # filepath : str (optional) input_params = {"transaction_id" : transaction_id, "user" : user, "group" : group, + "access_key" : access_key, + "secret_key" : secret_key, + "tenancy" : tenancy, "filepath" : filepath} # make the request @@ -475,6 +504,9 @@ def put_filelist(filelist: List[str]=[], config = load_config() user = get_user(config, user) group = get_group(config, group) + tenancy = get_tenancy(config) + access_key = get_access_key(config) + secret_key = get_secret_key(config) transaction_id = uuid.uuid4() url = construct_server_url(config) MAX_LOOPS = 2 @@ -505,16 +537,22 @@ def put_filelist(filelist: List[str]=[], "Authorization" : f"Bearer {auth_token['access_token']}" } - # build the parameters. files/put requires (for a filelist): # transaction_id: UUID # user: str # group: str + # access_key: str + # secret_key: str + # tenancy: str (optional) # and the filelist in the body input_params = {"transaction_id" : transaction_id, "user" : user, - "group" : group} + "group" : group, + "access_key" : access_key, + "secret_key" : secret_key, + "tenancy" : tenancy + } body_params = {"filelist" : filelist} # make the request try: diff --git a/nlds_client/templates/nlds-config.j2 b/nlds_client/templates/nlds-config.j2 index 209736a..5867516 100644 --- a/nlds_client/templates/nlds-config.j2 +++ b/nlds_client/templates/nlds-config.j2 @@ -1,11 +1,11 @@ { "server" : { - "url" : "http://127.0.0.1:8000", + "url" : "{{ nlds_server }}", "api" : "api/0.1" }, "user" : { - "default_user" : "nrmassey", - "default_group" : "cedaproc" + "default_user" : "{{ user }}", + "default_group" : "{{ group }}" }, "authentication" : { "oauth_client_id" : "{{ oauth_client_id }}", @@ -13,6 +13,12 @@ "oauth_token_url" : "{{ oauth_token_url }}", "oauth_scopes" : "{{ oauth_scopes }}", "oauth_token_file_location" : "~/.nlds-token" + }, + "object_storage" : { + "tenancy" : "{{ os_tenancy }}", + "access_key" : "{{ os_access_key }}", + "secret_key" : "{{ os_secret_key }}" + } "options" : { "verify_certificates": true From 2d6484521bb048b963dcf6c3be9712581234b40b Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Thu, 24 Mar 2022 12:26:22 +0000 Subject: [PATCH 07/49] Add fix to test and update template with options section --- nlds_client/templates/nlds-config.j2 | 2 +- tests/test_oauth.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nlds_client/templates/nlds-config.j2 b/nlds_client/templates/nlds-config.j2 index 5867516..adc5356 100644 --- a/nlds_client/templates/nlds-config.j2 +++ b/nlds_client/templates/nlds-config.j2 @@ -19,7 +19,7 @@ "access_key" : "{{ os_access_key }}", "secret_key" : "{{ os_secret_key }}" - } + }, "options" : { "verify_certificates": true } diff --git a/tests/test_oauth.py b/tests/test_oauth.py index d9aa947..d8ae985 100644 --- a/tests/test_oauth.py +++ b/tests/test_oauth.py @@ -83,7 +83,7 @@ def test_fetch_oauth_token(functional_config, test_value): @pytest.mark.parametrize("test_value", EDGE_VALUES) def test_fetch_oauth_config(monkeypatch, functional_config, test_value): - monkeypatch.setattr('save_token', None) + monkeypatch.setattr(clau, 'save_token', None) modified_config = copy.deepcopy(functional_config) modified_config['authentication']['oauth_client_id'] = test_value From ef0a834f6d3ffc62b4db0cec58d170858a22b6da Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Wed, 21 Sep 2022 09:54:08 +0100 Subject: [PATCH 08/49] Add target and source_transaction options to get commands --- nlds_client/clientlib/transactions.py | 62 +++++++++++++++++++-------- nlds_client/nlds_client.py | 12 ++++-- 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index a1cc219..b28471f 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -115,7 +115,8 @@ def process_transaction_response(response: requests.models.Response, url: str, ) -def get_file(filepath: str, user: str=None, group: str=None): +def get_file(filepath: str, user: str=None, group: str=None, + target: str = None, source_transact: str = None): """Make a request to get a single file from the NLDS. :param filepath: the path of the file to get from the storage :type filepath: string @@ -148,10 +149,19 @@ def get_file(filepath: str, user: str=None, group: str=None): url = construct_server_url(config) MAX_LOOPS = 2 + # If no target given then default to current working directory + if not target: + target = os.getcwd() + target_p = pathlib.Path(target) # Resolve path to file (i.e. make absolute) if configured so if get_option(config, "resolve_filenames"): # Convert to a pathlib.Path and then back to a string - filepath = str(pathlib.Path(filepath).resolve()) + # NB: No need to resolve the filepath as it should be verbatim what was + # in the original transaction? + target = str(target_p.resolve()) + # Recursively create the target path if it doesn't exist + if not target_p.exists(): + os.makedirs(target) while c_try < MAX_LOOPS: @@ -175,18 +185,22 @@ def get_file(filepath: str, user: str=None, group: str=None): } # build the parameters. files/get requires: - # transaction_id: UUID - # user: str - # group: str - # access_key : str - # secret_key : str - # tenancy : str (optional) + # transaction_id : UUID + # user : str + # group : str + # access_key : str + # secret_key : str + # tenancy : str (optional) + # target : str (optional - defaults to cwd) + # source_transact : str (optional) input_params = {"transaction_id" : transaction_id, "user" : user, "group" : group, "access_key" : access_key, "secret_key" : secret_key, "tenancy" : tenancy, + "target": target, + "source_transaction": source_transact, "filepath" : filepath} # make the request @@ -235,7 +249,8 @@ def get_file(filepath: str, user: str=None, group: str=None): return response_dict def get_filelist(filelist: List[str]=[], - user: str=None, group: str=None): + user: str=None, group: str=None, + target: str = None, source_transact: str = None) -> Dict: """Make a request to get a list of files from the NLDS. :param filelist: the list of filepaths to get from the storage :type filelist: List[string] @@ -270,13 +285,20 @@ def get_filelist(filelist: List[str]=[], url = construct_server_url(config) + "getlist" MAX_LOOPS = 2 + if not target: + target = os.getcwd() + target_p = pathlib.Path(target) # Resolve path to file (i.e. make absolute) if configured so if get_option(config, "resolve_filenames"): # Convert to a pathlib.Path and then back to a string - filelist = [str(pathlib.Path(fp).resolve()) for fp in filelist] + # NB: No need to resolve the filepath as it should be verbatim what was + # in the original transaction? + target = str(target_p.resolve()) + # Recursively create the target path if it doesn't exist + if not target_p.exists(): + os.makedirs(target) while c_try < MAX_LOOPS: - # get an OAuth token if we fail then the file doesn't exist. # we then fetch an Oauth2 token and try again c_try += 1 @@ -297,19 +319,23 @@ def get_filelist(filelist: List[str]=[], } # build the parameters. files/getlist/put requires: - # transaction_id: UUID - # user: str - # group: str - # access_key : str - # secret_key : str - # tenancy : str (optional) + # transaction_id : UUID + # user : str + # group : str + # access_key : str + # secret_key : str + # tenancy : str (optional) + # target : str (optional - defaults to cwd) + # source_transact : str (optional) # and the filelist in the body input_params = {"transaction_id" : transaction_id, "user" : user, "group" : group, "access_key" : access_key, "secret_key" : secret_key, - "tenancy" : tenancy + "tenancy" : tenancy, + "target": target, + "source_transaction": source_transact } body_params = {"filelist" : filelist} diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index b577d82..f2d966d 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -29,11 +29,13 @@ def put(filepath, user, group): """Get files command""" @click.option("--user", default=None, type=str) @click.option("--group", default=None, type=str) +@click.option("--target", default=None, type=click.Path()) +@click.option("--source_transact", default=None, type=str) @click.argument("filepath", type=str) @nlds_client.command() -def get(filepath, user, group): +def get(filepath, user, group, target, source_transact): try: - response = get_file(filepath, user, group) + response = get_file(filepath, user, group, target, source_transact) print(response) except ConnectionError as ce: raise click.UsageError(ce) @@ -71,9 +73,11 @@ def putlist(filelist, user, group): """Get filelist command""" @click.option("--user", default=None, type=str) @click.option("--group", default=None, type=str) +@click.option("--target", default=None, type=click.Path(exists=True)) +@click.option("--source_transact", default=None, type=str) @click.argument("filelist", type=str) @nlds_client.command() -def getlist(filelist, user, group): +def getlist(filelist, user, group, target, source_transact): # read the filelist from the file try: fh = open(filelist) @@ -83,7 +87,7 @@ def getlist(filelist, user, group): raise click.UsageError(fe) try: - response = get_filelist(files, user, group) + response = get_filelist(files, user, group, target, source_transact) print(response) except ConnectionError as ce: raise click.UsageError(ce) From 06b419190c3b3d089a2ba447284292eb3ea9731e Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Wed, 5 Oct 2022 14:20:25 +0100 Subject: [PATCH 09/49] Implement empty targets and resolution of filepaths The target can now be left empty and will be passed through to the NLDS as such, resulting in slightly different behaviour. No target will extract the files back to their original location, a given target will allow the files ot be placed somewhere else - this will be improved by future inclusion of common_path storage. Given filepaths in the no-target case will be resolved fully, the target is also reolved fully in the other case. --- nlds_client/clientlib/transactions.py | 83 ++++++++++++++++----------- nlds_client/nlds_client.py | 12 ++-- 2 files changed, 55 insertions(+), 40 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index b28471f..3f25db9 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -3,7 +3,7 @@ import uuid import urllib.parse import os -import pathlib +from pathlib import Path from typing import List, Dict @@ -116,7 +116,7 @@ def process_transaction_response(response: requests.models.Response, url: str, def get_file(filepath: str, user: str=None, group: str=None, - target: str = None, source_transact: str = None): + target: str = None, holding_transact: str = None): """Make a request to get a single file from the NLDS. :param filepath: the path of the file to get from the storage :type filepath: string @@ -149,19 +149,26 @@ def get_file(filepath: str, user: str=None, group: str=None, url = construct_server_url(config) MAX_LOOPS = 2 - # If no target given then default to current working directory - if not target: - target = os.getcwd() - target_p = pathlib.Path(target) - # Resolve path to file (i.e. make absolute) if configured so - if get_option(config, "resolve_filenames"): - # Convert to a pathlib.Path and then back to a string - # NB: No need to resolve the filepath as it should be verbatim what was - # in the original transaction? - target = str(target_p.resolve()) - # Recursively create the target path if it doesn't exist - if not target_p.exists(): - os.makedirs(target) + # If target given then we're operating in "mode 2" where we're downloading + # the file to a new location + if target: + target_p = Path(target) + # Resolve path to target (i.e. make absolute) if configured so + if get_option(config, "resolve_filenames"): + # Convert to a pathlib.Path to resolve and then back to a string + target = str(target_p.resolve()) + # Recursively create the target path if it doesn't exist + # NOTE: what permissions should be on this? Should _we_ be creating it + # here? or should we just error at this point? + if not target_p.exists(): + os.makedirs(target) + # If no target given then we are operating in "mode 1", i.e. we're + # downloading files back to their original locations. + else: + # Resolve path to file (i.e. make absolute) if configured so + if get_option(config, "resolve_filenames"): + # Convert to a pathlib.Path to resolve, and then back to a string + filepath = str(Path(filepath).resolve()) while c_try < MAX_LOOPS: @@ -192,7 +199,7 @@ def get_file(filepath: str, user: str=None, group: str=None, # secret_key : str # tenancy : str (optional) # target : str (optional - defaults to cwd) - # source_transact : str (optional) + # holding_transact : str (optional) input_params = {"transaction_id" : transaction_id, "user" : user, "group" : group, @@ -200,7 +207,7 @@ def get_file(filepath: str, user: str=None, group: str=None, "secret_key" : secret_key, "tenancy" : tenancy, "target": target, - "source_transaction": source_transact, + "holding_transaction": holding_transact, "filepath" : filepath} # make the request @@ -250,7 +257,7 @@ def get_file(filepath: str, user: str=None, group: str=None, def get_filelist(filelist: List[str]=[], user: str=None, group: str=None, - target: str = None, source_transact: str = None) -> Dict: + target: str = None, holding_transact: str = None) -> Dict: """Make a request to get a list of files from the NLDS. :param filelist: the list of filepaths to get from the storage :type filelist: List[string] @@ -285,18 +292,26 @@ def get_filelist(filelist: List[str]=[], url = construct_server_url(config) + "getlist" MAX_LOOPS = 2 - if not target: - target = os.getcwd() - target_p = pathlib.Path(target) - # Resolve path to file (i.e. make absolute) if configured so - if get_option(config, "resolve_filenames"): - # Convert to a pathlib.Path and then back to a string - # NB: No need to resolve the filepath as it should be verbatim what was - # in the original transaction? - target = str(target_p.resolve()) - # Recursively create the target path if it doesn't exist - if not target_p.exists(): - os.makedirs(target) + # If target given then we're operating in "mode 2" where we're downloading + # the file to a new location + if target: + target_p = Path(target) + # Resolve path to target (i.e. make absolute) if configured so + if get_option(config, "resolve_filenames"): + # Convert to a pathlib.Path to resolve and then back to a string + target = str(target_p.resolve()) + # Recursively create the target path if it doesn't exist + # NOTE: what permissions should be on this? Should _we_ be creating it + # here? or should we just error at this point? + if not target_p.exists(): + os.makedirs(target) + # If no target given then we are operating in "mode 1", i.e. we're + # downloading files back to their original locations. + else: + # Resolve path to file (i.e. make absolute) if configured so + if get_option(config, "resolve_filenames"): + # Convert to a pathlib.Path to resolve, and then back to a string + filelist = [str(Path(fp).resolve()) for fp in filelist] while c_try < MAX_LOOPS: # get an OAuth token if we fail then the file doesn't exist. @@ -326,7 +341,7 @@ def get_filelist(filelist: List[str]=[], # secret_key : str # tenancy : str (optional) # target : str (optional - defaults to cwd) - # source_transact : str (optional) + # holding_transact : str (optional) # and the filelist in the body input_params = {"transaction_id" : transaction_id, "user" : user, @@ -335,7 +350,7 @@ def get_filelist(filelist: List[str]=[], "secret_key" : secret_key, "tenancy" : tenancy, "target": target, - "source_transaction": source_transact + "holding_transaction": holding_transact } body_params = {"filelist" : filelist} @@ -420,7 +435,7 @@ def put_file(filepath: str, user: str=None, group: str=None, # Resolve path to file (i.e. make absolute) if configured so if get_option(config, "resolve_filenames"): # Convert to a pathlib.Path and then back to a string - filepath = str(pathlib.Path(filepath).resolve()) + filepath = str(Path(filepath).resolve()) MAX_LOOPS = 2 while c_try < MAX_LOOPS: @@ -540,7 +555,7 @@ def put_filelist(filelist: List[str]=[], # Resolve path to file (i.e. make absolute) if configured so if get_option(config, "resolve_filenames"): # Convert to a pathlib.Path and then back to a string - filelist = [str(pathlib.Path(fp).resolve()) for fp in filelist] + filelist = [str(Path(fp).resolve()) for fp in filelist] while c_try < MAX_LOOPS: diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index f2d966d..81adeff 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -30,12 +30,12 @@ def put(filepath, user, group): @click.option("--user", default=None, type=str) @click.option("--group", default=None, type=str) @click.option("--target", default=None, type=click.Path()) -@click.option("--source_transact", default=None, type=str) +@click.option("--holding_transact", default=None, type=str) @click.argument("filepath", type=str) @nlds_client.command() -def get(filepath, user, group, target, source_transact): +def get(filepath, user, group, target, holding_transact): try: - response = get_file(filepath, user, group, target, source_transact) + response = get_file(filepath, user, group, target, holding_transact) print(response) except ConnectionError as ce: raise click.UsageError(ce) @@ -74,10 +74,10 @@ def putlist(filelist, user, group): @click.option("--user", default=None, type=str) @click.option("--group", default=None, type=str) @click.option("--target", default=None, type=click.Path(exists=True)) -@click.option("--source_transact", default=None, type=str) +@click.option("--holding_transact", default=None, type=str) @click.argument("filelist", type=str) @nlds_client.command() -def getlist(filelist, user, group, target, source_transact): +def getlist(filelist, user, group, target, holding_transact): # read the filelist from the file try: fh = open(filelist) @@ -87,7 +87,7 @@ def getlist(filelist, user, group, target, source_transact): raise click.UsageError(fe) try: - response = get_filelist(files, user, group, target, source_transact) + response = get_filelist(files, user, group, target, holding_transact) print(response) except ConnectionError as ce: raise click.UsageError(ce) From 8ca7a2f2b17c43fc5ce40110d89764f860330bed Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Wed, 5 Oct 2022 14:59:23 +0100 Subject: [PATCH 10/49] Updated function documentation --- nlds_client/clientlib/transactions.py | 23 +++++++++++++---------- nlds_client/nlds_client.py | 6 +++--- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index b28471f..2f7da64 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -116,7 +116,7 @@ def process_transaction_response(response: requests.models.Response, url: str, def get_file(filepath: str, user: str=None, group: str=None, - target: str = None, source_transact: str = None): + target: str = None, holding_transaction_id: str = None): """Make a request to get a single file from the NLDS. :param filepath: the path of the file to get from the storage :type filepath: string @@ -124,6 +124,9 @@ def get_file(filepath: str, user: str=None, group: str=None, :type user: string, optional :param group: the group to get the file :type group: string + :param holding_transaction_id: the transaction id pertaining to the holding + containing the files + :type holding_transaction_id: string :raises requests.exceptions.ConnectionError: if the server cannot be reached @@ -185,14 +188,14 @@ def get_file(filepath: str, user: str=None, group: str=None, } # build the parameters. files/get requires: - # transaction_id : UUID - # user : str - # group : str - # access_key : str - # secret_key : str - # tenancy : str (optional) - # target : str (optional - defaults to cwd) - # source_transact : str (optional) + # transaction_id : UUID + # user : str + # group : str + # access_key : str + # secret_key : str + # tenancy : str (optional) + # target : str (optional - defaults to cwd) + # holding_transaction_id : str (optional) input_params = {"transaction_id" : transaction_id, "user" : user, "group" : group, @@ -200,7 +203,7 @@ def get_file(filepath: str, user: str=None, group: str=None, "secret_key" : secret_key, "tenancy" : tenancy, "target": target, - "source_transaction": source_transact, + "holding_transaction_id": holding_transaction_id, "filepath" : filepath} # make the request diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index f2d966d..5e81b1e 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -30,12 +30,12 @@ def put(filepath, user, group): @click.option("--user", default=None, type=str) @click.option("--group", default=None, type=str) @click.option("--target", default=None, type=click.Path()) -@click.option("--source_transact", default=None, type=str) +@click.option("--holding_transaction_id", default=None, type=str) @click.argument("filepath", type=str) @nlds_client.command() -def get(filepath, user, group, target, source_transact): +def get(filepath, user, group, target, holding_transaction_id): try: - response = get_file(filepath, user, group, target, source_transact) + response = get_file(filepath, user, group, target, holding_transaction_id) print(response) except ConnectionError as ce: raise click.UsageError(ce) From 16a34deb1cd329f40eed09419d436618389e55cd Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Sun, 30 Oct 2022 21:44:09 +0000 Subject: [PATCH 11/49] Updated client for get and put with labels --- nlds_client/clientlib/transactions.py | 316 ++++---------------------- nlds_client/nlds_client.py | 72 ++++-- setup.py | 2 +- 3 files changed, 100 insertions(+), 290 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index ba679c7..9d23dbd 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -115,152 +115,9 @@ def process_transaction_response(response: requests.models.Response, url: str, ) -def get_file(filepath: str, user: str=None, group: str=None, - target: str = None, holding_transaction_id: str = None): - """Make a request to get a single file from the NLDS. - :param filepath: the path of the file to get from the storage - :type filepath: string - :param user: the username to get the file - :type user: string, optional - :param group: the group to get the file - :type group: string - :param holding_transaction_id: the transaction id pertaining to the holding - containing the files - :type holding_transaction_id: string - - :raises requests.exceptions.ConnectionError: if the server cannot be - reached - - :return: A Dictionary of the response - :rtype: Dict - """ - - # try 3 times: - # 1. With token loaded from file at config["oauth_token_file_location"] - # 2. With a refresh token - # 3. Delete the token file - - c_try = 0 - # get the config, user and group - config = load_config() - user = get_user(config, user) - group = get_group(config, group) - tenancy = get_tenancy(config) - access_key = get_access_key(config) - secret_key = get_secret_key(config) - transaction_id = uuid.uuid4() - url = construct_server_url(config) - MAX_LOOPS = 2 - - # If target given then we're operating in "mode 2" where we're downloading - # the file to a new location - if target: - target_p = Path(target) - # Resolve path to target (i.e. make absolute) if configured so - if get_option(config, "resolve_filenames"): - # Convert to a pathlib.Path to resolve and then back to a string - target = str(target_p.resolve()) - # Recursively create the target path if it doesn't exist - # NOTE: what permissions should be on this? Should _we_ be creating it - # here? or should we just error at this point? - if not target_p.exists(): - os.makedirs(target) - # If no target given then we are operating in "mode 1", i.e. we're - # downloading files back to their original locations. - else: - # Resolve path to file (i.e. make absolute) if configured so - if get_option(config, "resolve_filenames"): - # Convert to a pathlib.Path to resolve, and then back to a string - filepath = str(Path(filepath).resolve()) - - while c_try < MAX_LOOPS: - - # get an OAuth token if we fail then the file doesn't exist. - # we then fetch an Oauth2 token and try again - c_try += 1 - try: - auth_token = load_token(config) - except FileNotFoundError: - # we need the username and password to get the OAuth2 token in - # the password flow - username, password = get_username_password(config) - auth_token = fetch_oauth2_token(config, username, password) - # we don't want to do the rest of the loop! - continue - - token_headers = { - "Content-Type" : "application/json", - "cache-control" : "no-cache", - "Authorization" : f"Bearer {auth_token['access_token']}" - } - - # build the parameters. files/get requires: - # transaction_id : UUID - # user : str - # group : str - # access_key : str - # secret_key : str - # tenancy : str (optional) - # target : str (optional - defaults to cwd) - # holding_transaction_id : str (optional) - input_params = {"transaction_id" : transaction_id, - "user" : user, - "group" : group, - "access_key" : access_key, - "secret_key" : secret_key, - "tenancy" : tenancy, - "target": target, - "holding_transaction_id": holding_transaction_id, - "filepath" : filepath} - - # make the request - try: - response = requests.get( - url, - headers = token_headers, - params = input_params, - verify = get_option(config, 'verify_certificates') - ) - except requests.exceptions.ConnectionError: - raise ConnectionError( - f"Could not connect to the URL: {url}\n" - "Check the ['server']['url'] and ['server']['api'] setting in " - f"the {CONFIG_FILE_LOCATION} file." - ) - - # process the returned response - try: - process_transaction_response(response, url, config) - except AuthenticationError: - # try to get a new token via the refresh method - try: - auth_token = fetch_oauth2_token_from_refresh(config) - continue - except (AuthenticationError, RequestError) as ae: - # delete the token file ready to try again! - if (ae.status_code == requests.codes.unauthorized or - ae.status_code == requests.codes.bad_request): - os.remove(os.path.expanduser( - config['authentication']['oauth_token_file_location'] - )) - continue - else: - raise ae - - response_dict = json.loads(response.json()) - response_dict['success'] = True - return response_dict - - # If we get to this point then the transaction could not be processed - response_dict = {'uuid' : str(transaction_id), - 'msg' : f'GET transaction with id {transaction_id} failed', - 'success' : False - } - return response_dict - def get_filelist(filelist: List[str]=[], - user: str=None, group: str=None, - target: str = None, holding_transact: str = None) -> Dict: + user: str=None, group: str=None, target: str = None, + label: str=None, holding_id: int=None, tag: dict=None) -> Dict: """Make a request to get a list of files from the NLDS. :param filelist: the list of filepaths to get from the storage :type filelist: List[string] @@ -271,8 +128,22 @@ def get_filelist(filelist: List[str]=[], :param group: the group to get the files :type group: string, optional - :raises requests.exceptions.ConnectionError: if the server cannot be - reached + :param target: the location to write the retrieved files to + :type target: string, optional + + :param label: the label of an existing holding that files are to be + retrieved from + :type label: str, optional + + :param holding_id: the integer id of a holding that files are to be + retrieved from + :type holding_id: int, optional + + :param tag: a dictionary of key:value pairs to search for in a holding that + files are to be retrieved from + :type tag: dict, optional + + :raises requests.exceptions.ConnectionError: if the server cannot be reached :return: A Dictionary of the response :rtype: Dict @@ -344,7 +215,6 @@ def get_filelist(filelist: List[str]=[], # secret_key : str # tenancy : str (optional) # target : str (optional - defaults to cwd) - # holding_transact : str (optional) # and the filelist in the body input_params = {"transaction_id" : transaction_id, "user" : user, @@ -353,10 +223,15 @@ def get_filelist(filelist: List[str]=[], "secret_key" : secret_key, "tenancy" : tenancy, "target": target, - "holding_transaction": holding_transact } body_params = {"filelist" : filelist} - + # add optional components to body: label, tags, holding_id + if label is not None: + body_params["label"] = label + if tag is not None: + body_params["tag"] = tag + if holding_id is not None: + body_params["holding_id"] = holding_id # make the request try: response = requests.put( @@ -404,128 +279,9 @@ def get_filelist(filelist: List[str]=[], return response_dict -def put_file(filepath: str, user: str=None, group: str=None, - ignore_certificates: bool=None): - """Make a request to put a single file into the NLDS. - - :param filepath: the path of the file to put into the storage - :type filepath: string - - :param user: the username to put the file - :type user: string, optional - - :param group: the group to put the file - :type group: string, optional - - :raises requests.exceptions.ConnectionError: if the server cannot be - reached - - :return: A Dictionary of the response - :rtype: Dict - """ - - c_try = 0 - # get the config, user and group - config = load_config() - user = get_user(config, user) - group = get_group(config, group) - tenancy = get_tenancy(config) - access_key = get_access_key(config) - secret_key = get_secret_key(config) - transaction_id = uuid.uuid4() - url = construct_server_url(config) - - # Resolve path to file (i.e. make absolute) if configured so - if get_option(config, "resolve_filenames"): - # Convert to a pathlib.Path and then back to a string - filepath = str(Path(filepath).resolve()) - - MAX_LOOPS = 2 - while c_try < MAX_LOOPS: - - # get an OAuth token if we fail then the file doesn't exist. - # we then fetch an Oauth2 token and try again - c_try += 1 - try: - auth_token = load_token(config) - except FileNotFoundError: - # we need the username and password to get the OAuth2 token in - # the password flow - username, password = get_username_password(config) - auth_token = fetch_oauth2_token(config, username, password) - # we don't want to do the rest of the loop! - continue - - token_headers = { - "Content-Type" : "application/json", - "cache-control" : "no-cache", - "Authorization" : f"Bearer {auth_token['access_token']}" - } - - # build the parameters. files/put requires: - # transaction_id: UUID - # user : str - # group : str - # access_key : str - # secret_key : str - # tenancy : str (optional) - # filepath : str (optional) - input_params = {"transaction_id" : transaction_id, - "user" : user, - "group" : group, - "access_key" : access_key, - "secret_key" : secret_key, - "tenancy" : tenancy, - "filepath" : filepath} - - # make the request - try: - response = requests.put( - url, - headers = token_headers, - params = input_params, - verify = get_option(config, 'verify_certificates') - ) - except requests.exceptions.ConnectionError: - raise ConnectionError( - f"Could not connect to the URL: {url}\n" - "Check the ['server']['url'] and ['server']['api'] setting in " - f"the {CONFIG_FILE_LOCATION} file." - ) - - # process the returned response - try: - process_transaction_response(response, url, config) - except AuthenticationError: - # try to get a new token via the refresh method - try: - auth_token = fetch_oauth2_token_from_refresh(config) - continue - except (AuthenticationError, RequestError) as ae: - # delete the token file ready to try again! - if (ae.status_code == requests.codes.unauthorized or - ae.status_code == requests.codes.bad_request): - os.remove(os.path.expanduser( - config['authentication']['oauth_token_file_location'] - )) - continue - else: - raise ae - - response_dict = json.loads(response.json()) - response_dict['success'] = True - return response_dict - - # If we get to this point then the transaction could not be processed - response_dict = {'uuid' : str(transaction_id), - 'msg' : f'PUT transaction with id {transaction_id} failed', - 'success' : False - } - return response_dict - - def put_filelist(filelist: List[str]=[], - user: str=None, group: str=None): + user: str=None, group: str=None, + label: str=None, holding_id: int=None, tag: dict=None): """Make a request to put a list of files into the NLDS. :param filelist: the list of filepaths to put into storage :type filelist: List[string] @@ -536,6 +292,15 @@ def put_filelist(filelist: List[str]=[], :param group: the group to put the files :type group: string + :param label: the label of an existing holding that files are to be added to + :type label: str, optional + + :param holding_id: the integer id of an existing holding that files are to be added to + :type holding_id: int, optional + + :param tag: a dictionary of key:value pairs to add as tags to the holding upon creation + :type tag: dict, optional + :raises requests.exceptions.ConnectionError: if the server cannot be reached @@ -598,8 +363,17 @@ def put_filelist(filelist: List[str]=[], "tenancy" : tenancy } body_params = {"filelist" : filelist} + # add optional components to body: label, tags, holding_id + if label is not None: + body_params["label"] = label + if tag is not None: + body_params["tag"] = tag + if holding_id is not None: + body_params["holding_id"] = holding_id # make the request try: + print(input_params) + print(body_params) response = requests.put( url, headers = token_headers, diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index dbc0859..7195468 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -1,7 +1,6 @@ #! /usr/bin/env python import click -from nlds_client.clientlib.transactions import get_file, get_filelist, \ - put_file, put_filelist +from nlds_client.clientlib.transactions import get_filelist, put_filelist from nlds_client.clientlib.exceptions import ConnectionError, RequestError, \ AuthenticationError @@ -9,14 +8,41 @@ def nlds_client(): pass + +"""Custom class for tags in the format key:value +(i.e. using the colon as separator between key and value.""" + +class TagParamType(click.ParamType): + name = "tag" + tag_dict = {} + + def convert(self, value, param, ctx): + try: + substrs = value.replace(" ","").split(",") + for s in substrs: + k, v = s.split(":") + self.tag_dict[k] = v + except ValueError: + self.fail( + f"{value!r} is not a valid (list of) tag(s)", param, ctx + ) + return self.tag_dict + +TAG_PARAM_TYPE = TagParamType() + + """Put files command""" +@nlds_client.command("put") @click.option("--user", default=None, type=str) @click.option("--group", default=None, type=str) +@click.option("--label", default=None, type=str) +@click.option("--holding_id", default=None, type=int) +@click.option("--tag", default=None, type=TAG_PARAM_TYPE) @click.argument("filepath", type=str) -@nlds_client.command() -def put(filepath, user, group): +def put(filepath, user, group, label, holding_id, tag): try: - response = put_file(filepath, user, group) + response = put_filelist([filepath], user, group, + label, holding_id, tag) print(response) except ConnectionError as ce: raise click.UsageError(ce) @@ -27,15 +53,18 @@ def put(filepath, user, group): """Get files command""" +@nlds_client.command("get") @click.option("--user", default=None, type=str) @click.option("--group", default=None, type=str) @click.option("--target", default=None, type=click.Path()) -@click.option("--holding_transaction_id", default=None, type=str) +@click.option("--label", default=None, type=str) +@click.option("--holding_id", default=None, type=int) +@click.option("--tag", default=None, type=TAG_PARAM_TYPE) @click.argument("filepath", type=str) -@nlds_client.command() -def get(filepath, user, group, target, holding_transaction_id): +def get(filepath, user, group, target, label, holding_id, tag): try: - response = get_file(filepath, user, group, target, holding_transaction_id) + response = get_filelist([filepath], user, group, target, + label, holding_id, tag) print(response) except ConnectionError as ce: raise click.UsageError(ce) @@ -46,11 +75,14 @@ def get(filepath, user, group, target, holding_transaction_id): """Put filelist command""" +@nlds_client.command("putlist") +@click.argument("filelist", type=str) @click.option("--user", default=None, type=str) @click.option("--group", default=None, type=str) -@click.argument("filelist", type=str) -@nlds_client.command() -def putlist(filelist, user, group): +@click.option("--label", default=None, type=str) +@click.option("--holding_id", default=None, type=int) +@click.option("--tag", default=None, type=TAG_PARAM_TYPE) +def putlist(filelist, user, group, label, holding_id, tag): # read the filelist from the file try: fh = open(filelist) @@ -60,7 +92,8 @@ def putlist(filelist, user, group): raise click.UsageError(fe) try: - response = put_filelist(files, user, group) + response = put_filelist(files, user, group, + label, holding_id, tag) print(response) except ConnectionError as ce: raise click.UsageError(ce) @@ -71,13 +104,15 @@ def putlist(filelist, user, group): """Get filelist command""" +@nlds_client.command("getlist") @click.option("--user", default=None, type=str) @click.option("--group", default=None, type=str) @click.option("--target", default=None, type=click.Path(exists=True)) -@click.option("--holding_transact", default=None, type=str) +@click.option("--label", default=None, type=str) +@click.option("--holding_id", default=None, type=int) +@click.option("--tag", default=None, type=TAG_PARAM_TYPE) @click.argument("filelist", type=str) -@nlds_client.command() -def getlist(filelist, user, group, target, holding_transact): +def getlist(filelist, user, group, target, label, holding_id, tag): # read the filelist from the file try: fh = open(filelist) @@ -87,7 +122,8 @@ def getlist(filelist, user, group, target, holding_transact): raise click.UsageError(fe) try: - response = get_filelist(files, user, group, target, holding_transact) + response = get_filelist(files, user, group, + target, holding_id, tag) print(response) except ConnectionError as ce: raise click.UsageError(ce) @@ -97,7 +133,7 @@ def getlist(filelist, user, group, target, holding_transact): raise click.UsageError(re) def main(): - nlds_client() + nlds_client(prog_name="nlds") if __name__ == "__main__": nlds_client() diff --git a/setup.py b/setup.py index 15dfc82..62ae4ad 100644 --- a/setup.py +++ b/setup.py @@ -9,7 +9,7 @@ setup( name='nlds-client', - version='0.0.1', + version='0.0.2', packages=['nlds_client'], install_requires=[ 'requests', From 753cbf39f9760f6617d1e4c6e8ab4bb75e05602c Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Mon, 7 Nov 2022 12:08:03 +0000 Subject: [PATCH 12/49] Changes to interface and transactions --- nlds_client/clientlib/transactions.py | 125 ++++++++++++++++++++++++-- nlds_client/nlds_client.py | 24 ++++- 2 files changed, 142 insertions(+), 7 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 9d23dbd..9175f9d 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -19,18 +19,21 @@ from nlds_client.clientlib.nlds_client_setup import CONFIG_FILE_LOCATION -def construct_server_url(config: Dict): +def construct_server_url(config: Dict, method=""): """Construct the url from the details in the config file. :param config: dictionary of key:value pairs returned from the load_config function. :type config: Dict + :param method: API method to call, currently either "files" or "holdings" + :type method: string + :return: a fully qualified url to the server hosting the REST API for NLDS. :rtype: string """ url = urllib.parse.urljoin(config["server"]["url"], "/".join([config["server"]["api"], - "files"]) + method]) ) + "/" return url @@ -163,7 +166,7 @@ def get_filelist(filelist: List[str]=[], access_key = get_access_key(config) secret_key = get_secret_key(config) transaction_id = uuid.uuid4() - url = construct_server_url(config) + "getlist" + url = construct_server_url(config, "files") + "getlist" MAX_LOOPS = 2 # If target given then we're operating in "mode 2" where we're downloading @@ -317,7 +320,7 @@ def put_filelist(filelist: List[str]=[], access_key = get_access_key(config) secret_key = get_secret_key(config) transaction_id = uuid.uuid4() - url = construct_server_url(config) + url = construct_server_url(config, "files") MAX_LOOPS = 2 # Resolve path to file (i.e. make absolute) if configured so @@ -372,8 +375,6 @@ def put_filelist(filelist: List[str]=[], body_params["holding_id"] = holding_id # make the request try: - print(input_params) - print(body_params) response = requests.put( url, headers = token_headers, @@ -417,3 +418,115 @@ def put_filelist(filelist: List[str]=[], 'success' : False } return response_dict + + +def list_holding(user, group, label, holding_id, tag): + """Make a request to list the holdings in the NLDS for a user + :param user: the username to get the holding(s) for + :type user: string + + :param group: the group to get the holding(s) for + :type group: string + + :param label: the label of an existing holding to get the details of + :type label: str, optional + + :param holding_id: the integer id of an existing holding to get the details of + :type holding_id: int, optional + + :param tag: a dictionary of key:value pairs to search holdings for - return holdings with these tags + :type tag: dict, optional + + :raises requests.exceptions.ConnectionError: if the server cannot be + reached + + :return: A Dictionary of the response + :rtype: Dict + """ + + c_try = 0 + # get the config, user and group + config = load_config() + user = get_user(config, user) + group = get_group(config, group) + tenancy = get_tenancy(config) + access_key = get_access_key(config) + secret_key = get_secret_key(config) + transaction_id = uuid.uuid4() + url = construct_server_url(config, "holdings") + MAX_LOOPS = 2 + + while c_try < MAX_LOOPS: + c_try += 1 + try: + auth_token = load_token(config) + except FileNotFoundError: + # we need the username and password to get the OAuth2 token in + # the password flow + username, password = get_username_password(config) + auth_token = fetch_oauth2_token(config, username, password) + # we don't want to do the rest of the loop! + continue + + token_headers = { + "Content-Type" : "application/json", + "cache-control" : "no-cache", + "Authorization" : f"Bearer {auth_token['access_token']}" + } + + # build the parameters. holdings->get requires + # user: str + # group: str + input_params = {"user" : user, + "group" : group} + + # add additional / optional components to input params + if label is not None: + input_params["label"] = label + if tag is not None: + input_params["tag"] = tag + if holding_id is not None: + input_params["holding_id"] = holding_id + # make the request + try: + response = requests.get( + url, + headers = token_headers, + params = input_params, + verify = get_option(config, 'verify_certificates') + ) + except requests.exceptions.ConnectionError: + raise ConnectionError( + f"Could not connect to the URL: {url}\n" + "Check the ['server']['url'] and ['server']['api'] setting in " + f"the {CONFIG_FILE_LOCATION} file." + ) + # process the returned response + try: + process_transaction_response(response, url, config) + except AuthenticationError: + # try to get a new token via the refresh method + try: + auth_token = fetch_oauth2_token_from_refresh(config) + continue + except (AuthenticationError, RequestError) as ae: + # delete the token file ready to try again! + if (ae.status_code == requests.codes.unauthorized or + ae.status_code == requests.codes.bad_request): + os.remove(os.path.expanduser( + config['authentication']['oauth_token_file_location'] + )) + continue + else: + raise ae + + response_dict = json.loads(response.json()) + response_dict['success'] = True + return response_dict + + # If we get to this point then the transaction could not be processed + response_dict = {'uuid' : str(transaction_id), + 'msg' : f'PUT transaction with id {transaction_id} failed', + 'success' : False + } + return response_dict \ No newline at end of file diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 7195468..245c439 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -1,6 +1,7 @@ #! /usr/bin/env python import click -from nlds_client.clientlib.transactions import get_filelist, put_filelist +from nlds_client.clientlib.transactions import get_filelist, put_filelist,\ + list_holding from nlds_client.clientlib.exceptions import ConnectionError, RequestError, \ AuthenticationError @@ -132,6 +133,27 @@ def getlist(filelist, user, group, target, label, holding_id, tag): except RequestError as re: raise click.UsageError(re) + +"""List (holdings) command""" +@nlds_client.command("list") +@click.option("--user", default=None, type=str) +@click.option("--group", default=None, type=str) +@click.option("--label", default=None, type=str) +@click.option("--holding_id", default=None, type=int) +@click.option("--tag", default=None, type=TAG_PARAM_TYPE) +def list(user, group, label, holding_id, tag): + # + try: + response = list_holding(user, group, label, holding_id, tag) + print(response) + except ConnectionError as ce: + raise click.UsageError(ce) + except AuthenticationError as ae: + raise click.UsageError(ae) + except RequestError as re: + raise click.UsageError(re) + + def main(): nlds_client(prog_name="nlds") From 0ba0ca08ca700f32c47e5190fbcc7a4b44499985 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 8 Nov 2022 16:59:41 +0000 Subject: [PATCH 13/49] Fixed tags in list call --- nlds_client/clientlib/transactions.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 9175f9d..88cf2d6 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -420,7 +420,8 @@ def put_filelist(filelist: List[str]=[], return response_dict -def list_holding(user, group, label, holding_id, tag): +def list_holding(user: str, group: str, + label: str, holding_id: int, tag: dict): """Make a request to list the holdings in the NLDS for a user :param user: the username to get the holding(s) for :type user: string @@ -428,13 +429,15 @@ def list_holding(user, group, label, holding_id, tag): :param group: the group to get the holding(s) for :type group: string - :param label: the label of an existing holding to get the details of + :param label: the label of an existing holding to get the details for :type label: str, optional - :param holding_id: the integer id of an existing holding to get the details of + :param holding_id: the integer id of an existing holding to get the details :type holding_id: int, optional - :param tag: a dictionary of key:value pairs to search holdings for - return holdings with these tags + :param tag: a list of key:value pairs to search holdings for - return + holdings with these tags. This will be converted to dictionary before + calling the remote method. :type tag: dict, optional :raises requests.exceptions.ConnectionError: if the server cannot be @@ -484,9 +487,14 @@ def list_holding(user, group, label, holding_id, tag): if label is not None: input_params["label"] = label if tag is not None: - input_params["tag"] = tag + # convert dict to string + tag_str = "" + for key in tag: + tag_str += key+":"+tag[key]+"," + input_params["tag"] = str(tag).replace("'","") if holding_id is not None: input_params["holding_id"] = holding_id + # make the request try: response = requests.get( From fc7cc0d94b59f695987e14b936141fd4a95291f3 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Thu, 10 Nov 2022 16:34:40 +0000 Subject: [PATCH 14/49] Actually output something from request --- nlds_client/nlds_client.py | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 245c439..4c94d0c 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -4,6 +4,7 @@ list_holding from nlds_client.clientlib.exceptions import ConnectionError, RequestError, \ AuthenticationError +from nlds_client.clientlib.config import get_user, get_group, load_config @click.group() def nlds_client(): @@ -133,6 +134,31 @@ def getlist(filelist, user, group, target, label, holding_id, tag): except RequestError as re: raise click.UsageError(re) +def format_request_details(user, group, label, holding_id, tag): + config = load_config() + out = "" + user = get_user(config, user) + out += f"user: {user}, " + + group = get_group(config, group) + out += f"group: {group}, " + + if label: + out += f"label: {label}, " + if holding_id: + out += f"holding_id: {holding_id}, " + if tag: + out += f"tag: {tag}, " + return out[:-2] + +def print_list(response: dict, req_details): + """Print out the response from the list command""" + list_string = "Listing holding(s) for: " + list_string += req_details + print(list_string) + for h in response['data']['holdings']: + print(h) + """List (holdings) command""" @nlds_client.command("list") @@ -145,7 +171,16 @@ def list(user, group, label, holding_id, tag): # try: response = list_holding(user, group, label, holding_id, tag) - print(response) + req_details = format_request_details( + user, group, label, holding_id, tag + ) + if response['success'] and len(response['data']['holdings']) > 0: + print_list(response, req_details) + else: + fail_string = "Failed to list holding with " + fail_string += req_details + raise click.UsageError(fail_string) + except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: From 9b5b434548da8a0e0b099947d93dd51eb1686397 Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Mon, 14 Nov 2022 17:24:50 +0000 Subject: [PATCH 15/49] Add stat command functionality --- nlds_client/clientlib/transactions.py | 118 ++++++++++++++++++++++++++ nlds_client/nlds_client.py | 24 +++++- 2 files changed, 140 insertions(+), 2 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 88cf2d6..340be07 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -537,4 +537,122 @@ def list_holding(user: str, group: str, 'msg' : f'PUT transaction with id {transaction_id} failed', 'success' : False } + return response_dict + + +def monitor_transactions(user, group, transaction_id, sub_id, state, + retry_count): + """Make a request to the monitoring database for a status update of ongoing + or finished transactions in the NLDS for, a user/group + + :param user: the username to get the transaction state(s) for + :type user: string + + :param group: the group to get the transaction state(s) for + :type group: string + + :param transaction_id: a specific transaction_id to get the status of + :type transaction_id: string, optional + + :param sub_id: a specific sub_id (of a sub_record) to get the status of + :type sub_id: string, optional + + :param state: applies a state-specific filter to the status request, only + sub_records at the given state will be returned. + :type state: string or int, optional + + :param retry_count: returns sub_records at the given retry_count value + :type retry_count: int, optional + + :raises requests.exceptions.ConnectionError: if the server cannot be + reached + + :return: A Dictionary of the response + :rtype: Dict + """ + + c_try = 0 + # get the config, user and group + config = load_config() + user = get_user(config, user) + group = get_group(config, group) + url = construct_server_url(config, "status") + MAX_LOOPS = 2 + + while c_try < MAX_LOOPS: + c_try += 1 + try: + auth_token = load_token(config) + except FileNotFoundError: + # we need the username and password to get the OAuth2 token in + # the password flow + username, password = get_username_password(config) + auth_token = fetch_oauth2_token(config, username, password) + # we don't want to do the rest of the loop! + continue + + token_headers = { + "Content-Type" : "application/json", + "cache-control" : "no-cache", + "Authorization" : f"Bearer {auth_token['access_token']}" + } + + # build the parameters. monitoring->get requires + # user: str + # group: str + input_params = {"user" : user, + "group" : group} + + # add additional / optional components to input params + if transaction_id is not None: + input_params["transaction_id"] = transaction_id + if sub_id is not None: + input_params["sub_id"] = sub_id + if state is not None: + input_params["state"] = state + if retry_count is not None: + input_params["retry_count"] = retry_count + + # make the request + try: + response = requests.get( + url, + headers = token_headers, + params = input_params, + verify = get_option(config, 'verify_certificates') + ) + except requests.exceptions.ConnectionError: + raise ConnectionError( + f"Could not connect to the URL: {url}\n" + "Check the ['server']['url'] and ['server']['api'] setting in " + f"the {CONFIG_FILE_LOCATION} file." + ) + # process the returned response + try: + process_transaction_response(response, url, config) + except AuthenticationError: + # try to get a new token via the refresh method + try: + auth_token = fetch_oauth2_token_from_refresh(config) + continue + except (AuthenticationError, RequestError) as ae: + # delete the token file ready to try again! + if (ae.status_code == requests.codes.unauthorized or + ae.status_code == requests.codes.bad_request): + os.remove(os.path.expanduser( + config['authentication']['oauth_token_file_location'] + )) + continue + else: + raise ae + + response_dict = json.loads(response.json()) + response_dict['success'] = True + return response_dict + + # If we get to this point then the transaction could not be processed + response_dict = { + "msg": f"GET transaction for user {user} and group {group} failed", + "success": False + } return response_dict \ No newline at end of file diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 4c94d0c..399dd73 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -1,7 +1,8 @@ #! /usr/bin/env python import click -from nlds_client.clientlib.transactions import get_filelist, put_filelist,\ - list_holding +from nlds_client.clientlib.transactions import (get_filelist, put_filelist, + list_holding, + monitor_transactions) from nlds_client.clientlib.exceptions import ConnectionError, RequestError, \ AuthenticationError from nlds_client.clientlib.config import get_user, get_group, load_config @@ -188,6 +189,25 @@ def list(user, group, label, holding_id, tag): except RequestError as re: raise click.UsageError(re) +"""Stat (monitoring) command""" +@nlds_client.command("stat") +@click.option("--user", default=None, type=str) +@click.option("--group", default=None, type=str) +@click.option("--transaction_id", default=None, type=str) +@click.option("--sub_id", default=None, type=str) +@click.option("--state", default=None, type=(str, int)) +@click.option("--retry_count", default=None, type=int) +def stat(user, group, transaction_id, sub_id, state, retry_count): + try: + response = monitor_transactions(user, group, transaction_id, sub_id, state, + retry_count) + print(response) + except ConnectionError as ce: + raise click.UsageError(ce) + except AuthenticationError as ae: + raise click.UsageError(ae) + except RequestError as re: + raise click.UsageError(re) def main(): nlds_client(prog_name="nlds") From 30a8beb91fb6e58434edb1b7fd1d53fc907efb2e Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 15 Nov 2022 07:59:12 +0000 Subject: [PATCH 16/49] Major refactor of the command functions - lots of copy paste code removed --- nlds_client/clientlib/transactions.py | 460 +++++++++++--------------- nlds_client/nlds_client.py | 34 +- 2 files changed, 226 insertions(+), 268 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 88cf2d6..4927e22 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -118,81 +118,32 @@ def process_transaction_response(response: requests.models.Response, url: str, ) -def get_filelist(filelist: List[str]=[], - user: str=None, group: str=None, target: str = None, - label: str=None, holding_id: int=None, tag: dict=None) -> Dict: - """Make a request to get a list of files from the NLDS. - :param filelist: the list of filepaths to get from the storage - :type filelist: List[string] - - :param user: the username to get the files - :type user: string, optional - - :param group: the group to get the files - :type group: string, optional - - :param target: the location to write the retrieved files to - :type target: string, optional - - :param label: the label of an existing holding that files are to be - retrieved from - :type label: str, optional +def main_loop(url: str, + input_params: dict={}, + body_params: dict={}, + method=requests.get): + """Generalised main loop to make requests to the NLDS server + :param url: the API URL to contact + :type user: string - :param holding_id: the integer id of a holding that files are to be - retrieved from - :type holding_id: int, optional + :param input_params: the input parameters for the API request (or {}) + :type input_params: dict - :param tag: a dictionary of key:value pairs to search for in a holding that - files are to be retrieved from - :type tag: dict, optional + :param body_params: the body parameters for the API request (or {}) + :type body_params: dict - :raises requests.exceptions.ConnectionError: if the server cannot be reached + :raises requests.exceptions.ConnectionError: if the server cannot be + reached - :return: A Dictionary of the response + :return: A Dictionary of the response or None :rtype: Dict """ - # try 3 times: - # 1. With token loaded from file at config["oauth_token_file_location"] - # 2. With a refresh token - # 3. Delete the token file - + config = load_config() c_try = 0 - # get the config, user and group - config = load_config() - user = get_user(config, user) - group = get_group(config, group) - tenancy = get_tenancy(config) - access_key = get_access_key(config) - secret_key = get_secret_key(config) - transaction_id = uuid.uuid4() - url = construct_server_url(config, "files") + "getlist" MAX_LOOPS = 2 - - # If target given then we're operating in "mode 2" where we're downloading - # the file to a new location - if target: - target_p = Path(target) - # Resolve path to target (i.e. make absolute) if configured so - if get_option(config, "resolve_filenames"): - # Convert to a pathlib.Path to resolve and then back to a string - target = str(target_p.resolve()) - # Recursively create the target path if it doesn't exist - # NOTE: what permissions should be on this? Should _we_ be creating it - # here? or should we just error at this point? - if not target_p.exists(): - os.makedirs(target) - # If no target given then we are operating in "mode 1", i.e. we're - # downloading files back to their original locations. - else: - # Resolve path to file (i.e. make absolute) if configured so - if get_option(config, "resolve_filenames"): - # Convert to a pathlib.Path to resolve, and then back to a string - filelist = [str(Path(fp).resolve()) for fp in filelist] - + while c_try < MAX_LOOPS: - # get an OAuth token if we fail then the file doesn't exist. - # we then fetch an Oauth2 token and try again c_try += 1 try: auth_token = load_token(config) @@ -210,34 +161,9 @@ def get_filelist(filelist: List[str]=[], "Authorization" : f"Bearer {auth_token['access_token']}" } - # build the parameters. files/getlist/put requires: - # transaction_id : UUID - # user : str - # group : str - # access_key : str - # secret_key : str - # tenancy : str (optional) - # target : str (optional - defaults to cwd) - # and the filelist in the body - input_params = {"transaction_id" : transaction_id, - "user" : user, - "group" : group, - "access_key" : access_key, - "secret_key" : secret_key, - "tenancy" : tenancy, - "target": target, - } - body_params = {"filelist" : filelist} - # add optional components to body: label, tags, holding_id - if label is not None: - body_params["label"] = label - if tag is not None: - body_params["tag"] = tag - if holding_id is not None: - body_params["holding_id"] = holding_id # make the request try: - response = requests.put( + response = method( url, headers = token_headers, params = input_params, @@ -250,7 +176,6 @@ def get_filelist(filelist: List[str]=[], "Check the ['server']['url'] and ['server']['api'] setting in " f"the {CONFIG_FILE_LOCATION} file." ) - # process the returned response try: process_transaction_response(response, url, config) @@ -275,11 +200,7 @@ def get_filelist(filelist: List[str]=[], return response_dict # If we get to this point then the transaction could not be processed - response_dict = {'uuid' : str(transaction_id), - 'msg' : f'GETLIST transaction with id {transaction_id} failed', - 'success' : False - } - return response_dict + return None def put_filelist(filelist: List[str]=[], @@ -311,112 +232,167 @@ def put_filelist(filelist: List[str]=[], :rtype: Dict """ - c_try = 0 # get the config, user and group config = load_config() user = get_user(config, user) group = get_group(config, group) + url = construct_server_url(config, "files") tenancy = get_tenancy(config) access_key = get_access_key(config) secret_key = get_secret_key(config) transaction_id = uuid.uuid4() - url = construct_server_url(config, "files") - MAX_LOOPS = 2 - + # Resolve path to file (i.e. make absolute) if configured so if get_option(config, "resolve_filenames"): # Convert to a pathlib.Path and then back to a string filelist = [str(Path(fp).resolve()) for fp in filelist] - while c_try < MAX_LOOPS: + # build the parameters. files/put requires (for a filelist): + # transaction_id: UUID + # user: str + # group: str + # access_key: str + # secret_key: str + # tenancy: str (optional) + # and the filelist in the body + + input_params = {"transaction_id" : transaction_id, + "user" : user, + "group" : group, + "access_key" : access_key, + "secret_key" : secret_key, + "tenancy" : tenancy + } + + body_params = {"filelist" : filelist} + # add optional components to body: label, tags, holding_id + if label is not None: + body_params["label"] = label + if tag is not None: + body_params["tag"] = tag + if holding_id is not None: + body_params["holding_id"] = holding_id + # make the request + response_dict = main_loop( + url=url, + input_params=input_params, + body_params=body_params, + method=requests.put + ) + + if not response_dict: + # If we get to this point then the transaction could not be processed + response_dict = {'uuid' : str(transaction_id), + 'msg' : f'PUT transaction with id {transaction_id} failed', + 'success' : False + } + return response_dict - # get an OAuth token if we fail then the file doesn't exist. - # we then fetch an Oauth2 token and try again - c_try += 1 - try: - auth_token = load_token(config) - except FileNotFoundError: - # we need the username and password to get the OAuth2 token in - # the password flow - username, password = get_username_password(config) - auth_token = fetch_oauth2_token(config, username, password) - # we don't want to do the rest of the loop! - continue - token_headers = { - "Content-Type" : "application/json", - "cache-control" : "no-cache", - "Authorization" : f"Bearer {auth_token['access_token']}" - } +def get_filelist(filelist: List[str]=[], + user: str=None, group: str=None, target: str = None, + label: str=None, holding_id: int=None, tag: dict=None) -> Dict: + """Make a request to get a list of files from the NLDS. + :param filelist: the list of filepaths to get from the storage + :type filelist: List[string] - # build the parameters. files/put requires (for a filelist): - # transaction_id: UUID - # user: str - # group: str - # access_key: str - # secret_key: str - # tenancy: str (optional) - # and the filelist in the body - - input_params = {"transaction_id" : transaction_id, - "user" : user, - "group" : group, - "access_key" : access_key, - "secret_key" : secret_key, - "tenancy" : tenancy - } - body_params = {"filelist" : filelist} - # add optional components to body: label, tags, holding_id - if label is not None: - body_params["label"] = label - if tag is not None: - body_params["tag"] = tag - if holding_id is not None: - body_params["holding_id"] = holding_id - # make the request - try: - response = requests.put( - url, - headers = token_headers, - params = input_params, - json = body_params, - verify = get_option(config, 'verify_certificates') - ) - except requests.exceptions.ConnectionError: - raise ConnectionError( - f"Could not connect to the URL: {url}\n" - "Check the ['server']['url'] and ['server']['api'] setting in " - f"the {CONFIG_FILE_LOCATION} file." - ) + :param user: the username to get the files + :type user: string, optional - # process the returned response - try: - process_transaction_response(response, url, config) - except AuthenticationError: - # try to get a new token via the refresh method - try: - auth_token = fetch_oauth2_token_from_refresh(config) - continue - except (AuthenticationError, RequestError) as ae: - # delete the token file ready to try again! - if (ae.status_code == requests.codes.unauthorized or - ae.status_code == requests.codes.bad_request): - os.remove(os.path.expanduser( - config['authentication']['oauth_token_file_location'] - )) - continue - else: - raise ae + :param group: the group to get the files + :type group: string, optional - response_dict = json.loads(response.json()) - response_dict['success'] = True - return response_dict + :param target: the location to write the retrieved files to + :type target: string, optional - # If we get to this point then the transaction could not be processed - response_dict = {'uuid' : str(transaction_id), - 'msg' : f'PUT transaction with id {transaction_id} failed', - 'success' : False - } + :param label: the label of an existing holding that files are to be + retrieved from + :type label: str, optional + + :param holding_id: the integer id of a holding that files are to be + retrieved from + :type holding_id: int, optional + + :param tag: a dictionary of key:value pairs to search for in a holding that + files are to be retrieved from + :type tag: dict, optional + + :raises requests.exceptions.ConnectionError: if the server cannot be reached + + :return: A Dictionary of the response + :rtype: Dict + """ + + # get the config, user and group + config = load_config() + user = get_user(config, user) + group = get_group(config, group) + tenancy = get_tenancy(config) + access_key = get_access_key(config) + secret_key = get_secret_key(config) + transaction_id = uuid.uuid4() + url = construct_server_url(config, "files") + "getlist" + + # If target given then we're operating in "mode 2" where we're downloading + # the file to a new location + if target: + target_p = Path(target) + # Resolve path to target (i.e. make absolute) if configured so + if get_option(config, "resolve_filenames"): + # Convert to a pathlib.Path to resolve and then back to a string + target = str(target_p.resolve()) + # Recursively create the target path if it doesn't exist + # NOTE: what permissions should be on this? Should _we_ be creating it + # here? or should we just error at this point? + if not target_p.exists(): + os.makedirs(target) + # If no target given then we are operating in "mode 1", i.e. we're + # downloading files back to their original locations. + else: + # Resolve path to file (i.e. make absolute) if configured so + if get_option(config, "resolve_filenames"): + # Convert to a pathlib.Path to resolve, and then back to a string + filelist = [str(Path(fp).resolve()) for fp in filelist] + + # build the parameters. files/getlist/put requires: + # transaction_id : UUID + # user : str + # group : str + # access_key : str + # secret_key : str + # tenancy : str (optional) + # target : str (optional - defaults to cwd) + # and the filelist in the body + input_params = {"transaction_id" : transaction_id, + "user" : user, + "group" : group, + "access_key" : access_key, + "secret_key" : secret_key, + "tenancy" : tenancy, + "target": target, + } + body_params = {"filelist" : filelist} + # add optional components to body: label, tags, holding_id + if label is not None: + body_params["label"] = label + if tag is not None: + body_params["tag"] = tag + if holding_id is not None: + body_params["holding_id"] = holding_id + # make the request + response_dict = main_loop( + url=url, + input_params=input_params, + body_params=body_params, + method=requests.put + ) + + if not response_dict: + # If we get to this point then the transaction could not be processed + response_dict = {'uuid' : str(transaction_id), + 'msg' : f'GET transaction with id {transaction_id} failed', + 'success' : False + } return response_dict @@ -447,94 +423,44 @@ def list_holding(user: str, group: str, :rtype: Dict """ - c_try = 0 # get the config, user and group config = load_config() user = get_user(config, user) group = get_group(config, group) - tenancy = get_tenancy(config) - access_key = get_access_key(config) - secret_key = get_secret_key(config) - transaction_id = uuid.uuid4() url = construct_server_url(config, "holdings") - MAX_LOOPS = 2 - - while c_try < MAX_LOOPS: - c_try += 1 - try: - auth_token = load_token(config) - except FileNotFoundError: - # we need the username and password to get the OAuth2 token in - # the password flow - username, password = get_username_password(config) - auth_token = fetch_oauth2_token(config, username, password) - # we don't want to do the rest of the loop! - continue + transaction_id = uuid.uuid4() - token_headers = { - "Content-Type" : "application/json", - "cache-control" : "no-cache", - "Authorization" : f"Bearer {auth_token['access_token']}" + # build the parameters. holdings->get requires + # user: str + # group: str + input_params = {"user" : user, + "group" : group} + + # add additional / optional components to input params + if label is not None: + input_params["label"] = label + if tag is not None: + # convert dict to string + tag_str = "" + for key in tag: + tag_str += key+":"+tag[key]+"," + input_params["tag"] = str(tag).replace("'","") + if holding_id is not None: + input_params["holding_id"] = holding_id + + response_dict = main_loop( + url=url, + input_params=input_params, + method=requests.get + ) + + if not response_dict: + response_dict = { + 'uuid' : str(transaction_id), + 'msg' : f'LIST holdings with id {transaction_id} failed', + 'success' : False } + return response_dict - # build the parameters. holdings->get requires - # user: str - # group: str - input_params = {"user" : user, - "group" : group} - - # add additional / optional components to input params - if label is not None: - input_params["label"] = label - if tag is not None: - # convert dict to string - tag_str = "" - for key in tag: - tag_str += key+":"+tag[key]+"," - input_params["tag"] = str(tag).replace("'","") - if holding_id is not None: - input_params["holding_id"] = holding_id - - # make the request - try: - response = requests.get( - url, - headers = token_headers, - params = input_params, - verify = get_option(config, 'verify_certificates') - ) - except requests.exceptions.ConnectionError: - raise ConnectionError( - f"Could not connect to the URL: {url}\n" - "Check the ['server']['url'] and ['server']['api'] setting in " - f"the {CONFIG_FILE_LOCATION} file." - ) - # process the returned response - try: - process_transaction_response(response, url, config) - except AuthenticationError: - # try to get a new token via the refresh method - try: - auth_token = fetch_oauth2_token_from_refresh(config) - continue - except (AuthenticationError, RequestError) as ae: - # delete the token file ready to try again! - if (ae.status_code == requests.codes.unauthorized or - ae.status_code == requests.codes.bad_request): - os.remove(os.path.expanduser( - config['authentication']['oauth_token_file_location'] - )) - continue - else: - raise ae - - response_dict = json.loads(response.json()) - response_dict['success'] = True - return response_dict - - # If we get to this point then the transaction could not be processed - response_dict = {'uuid' : str(transaction_id), - 'msg' : f'PUT transaction with id {transaction_id} failed', - 'success' : False - } - return response_dict \ No newline at end of file +def find_file(): + pass \ No newline at end of file diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 4c94d0c..9bd0520 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -1,7 +1,7 @@ #! /usr/bin/env python import click from nlds_client.clientlib.transactions import get_filelist, put_filelist,\ - list_holding + list_holding, find_file from nlds_client.clientlib.exceptions import ConnectionError, RequestError, \ AuthenticationError from nlds_client.clientlib.config import get_user, get_group, load_config @@ -159,6 +159,8 @@ def print_list(response: dict, req_details): for h in response['data']['holdings']: print(h) +def print_find(response:dict, req_details): + """Print out the response from the find command""" """List (holdings) command""" @nlds_client.command("list") @@ -189,6 +191,36 @@ def list(user, group, label, holding_id, tag): raise click.UsageError(re) +"""Find (files) command""" +@nlds_client.command("find") +@click.option("--user", default=None, type=str) +@click.option("--group", default=None, type=str) +@click.option("--label", default=None, type=str) +@click.option("--holding_id", default=None, type=int) +@click.option("--path", default=None, type=str) +@click.option("--tag", default=None, type=TAG_PARAM_TYPE) +def list(user, group, label, holding_id, tag): + # + try: + response = find_file(user, group, label, holding_id, path, tag) + req_details = format_request_details( + user, group, label, holding_id, tag + ) + if response['success'] and len(response['data']['holdings']) > 0: + print_find(response, req_details) + else: + fail_string = "Failed to list files with " + fail_string += req_details + raise click.UsageError(fail_string) + + except ConnectionError as ce: + raise click.UsageError(ce) + except AuthenticationError as ae: + raise click.UsageError(ae) + except RequestError as re: + raise click.UsageError(re) + + def main(): nlds_client(prog_name="nlds") From 87bc8f9a07e8d501f51bc7df2c68d03d45abf54e Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 15 Nov 2022 10:07:23 +0000 Subject: [PATCH 17/49] Refactored stat to use new main_loop function --- nlds_client/clientlib/transactions.py | 122 ++++++++------------------ nlds_client/nlds_client.py | 34 +++++-- 2 files changed, 65 insertions(+), 91 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 8d424cc..cf44240 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -63,6 +63,7 @@ def process_transaction_response(response: requests.models.Response, url: str, triggered. :rtype: requests.models.Response object """ + print(response.status_code) # possible responses: 202, 400, 403, 404, 422. try: if (response.status_code == requests.codes.ok or @@ -92,17 +93,18 @@ def process_transaction_response(response: requests.models.Response, url: str, response.status_code ) elif (response.status_code == requests.codes.not_found): # 404 - response_json = json.loads(response.json()['detail']) + print(response.content) + response_msg = response.json()['detail'] raise RequestError( f"Could not complete the request to the URL: {url} \n" - f"{response_json['msg']} (HTTP_{response.status_code})", + f"{response_msg} (HTTP_{response.status_code})", response.status_code ) elif (response.status_code == requests.codes.unprocessable): # 422 - response_json = response.json()['detail'][0] + response_msg = response.json()['detail'] raise RequestError( f"Could not complete the request to the URL: {url} \n" - f"{response_json['msg']} (HTTP_{response.status_code})", + f"{response_msg} (HTTP_{response.status_code})", response.status_code ) else: @@ -389,10 +391,11 @@ def get_filelist(filelist: List[str]=[], if not response_dict: # If we get to this point then the transaction could not be processed - response_dict = {'uuid' : str(transaction_id), - 'msg' : f'GET transaction with id {transaction_id} failed', - 'success' : False - } + response_dict = { + "uuid" : str(transaction_id), + "msg" : f"GET transaction with id {transaction_id} failed", + "success" : False + } return response_dict @@ -428,7 +431,6 @@ def list_holding(user: str, group: str, user = get_user(config, user) group = get_group(config, group) url = construct_server_url(config, "holdings") - transaction_id = uuid.uuid4() # build the parameters. holdings->get requires # user: str @@ -456,9 +458,8 @@ def list_holding(user: str, group: str, if not response_dict: response_dict = { - 'uuid' : str(transaction_id), - 'msg' : f'LIST holdings with id {transaction_id} failed', - 'success' : False + "msg" : f"LIST holdings for user {user} and group {group} failed", + "success" : False } return response_dict @@ -498,88 +499,39 @@ def monitor_transactions(user, group, transaction_id, sub_id, state, :rtype: Dict """ - c_try = 0 # get the config, user and group config = load_config() user = get_user(config, user) group = get_group(config, group) url = construct_server_url(config, "status") MAX_LOOPS = 2 - - while c_try < MAX_LOOPS: - c_try += 1 - try: - auth_token = load_token(config) - except FileNotFoundError: - # we need the username and password to get the OAuth2 token in - # the password flow - username, password = get_username_password(config) - auth_token = fetch_oauth2_token(config, username, password) - # we don't want to do the rest of the loop! - continue - - token_headers = { - "Content-Type" : "application/json", - "cache-control" : "no-cache", - "Authorization" : f"Bearer {auth_token['access_token']}" - } - # build the parameters. monitoring->get requires - # user: str - # group: str - input_params = {"user" : user, - "group" : group} - - # add additional / optional components to input params - if transaction_id is not None: - input_params["transaction_id"] = transaction_id - if sub_id is not None: - input_params["sub_id"] = sub_id - if state is not None: - input_params["state"] = state - if retry_count is not None: - input_params["retry_count"] = retry_count + # build the parameters. monitoring->get requires + # user: str + # group: str + input_params = {"user" : user, + "group" : group} - # make the request - try: - response = requests.get( - url, - headers = token_headers, - params = input_params, - verify = get_option(config, 'verify_certificates') - ) - except requests.exceptions.ConnectionError: - raise ConnectionError( - f"Could not connect to the URL: {url}\n" - "Check the ['server']['url'] and ['server']['api'] setting in " - f"the {CONFIG_FILE_LOCATION} file." - ) - # process the returned response - try: - process_transaction_response(response, url, config) - except AuthenticationError: - # try to get a new token via the refresh method - try: - auth_token = fetch_oauth2_token_from_refresh(config) - continue - except (AuthenticationError, RequestError) as ae: - # delete the token file ready to try again! - if (ae.status_code == requests.codes.unauthorized or - ae.status_code == requests.codes.bad_request): - os.remove(os.path.expanduser( - config['authentication']['oauth_token_file_location'] - )) - continue - else: - raise ae + # add additional / optional components to input params + if transaction_id is not None: + input_params["transaction_id"] = transaction_id + if sub_id is not None: + input_params["sub_id"] = sub_id + if state is not None: + input_params["state"] = state + if retry_count is not None: + input_params["retry_count"] = retry_count - response_dict = json.loads(response.json()) - response_dict['success'] = True - return response_dict + response_dict = main_loop( + url=url, + input_params=input_params, + method=requests.get + ) # If we get to this point then the transaction could not be processed - response_dict = { - "msg": f"GET transaction for user {user} and group {group} failed", - "success": False - } + if not response_dict: + response_dict = { + "msg": f"STAT transaction for user {user} and group {group} failed", + "success": False + } return response_dict diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index f10090e..1a2973a 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -1,7 +1,7 @@ #! /usr/bin/env python import click from nlds_client.clientlib.transactions import (get_filelist, put_filelist, - list_holding, + list_holding, find_file, monitor_transactions) from nlds_client.clientlib.exceptions import ConnectionError, RequestError, \ AuthenticationError @@ -135,7 +135,9 @@ def getlist(filelist, user, group, target, label, holding_id, tag): except RequestError as re: raise click.UsageError(re) -def format_request_details(user, group, label, holding_id, tag): +def format_request_details(user, group, label=None, holding_id=None, + tag=None, transaction_id=None, sub_id=None, + state=None, retry_count=None): config = load_config() out = "" user = get_user(config, user) @@ -150,6 +152,14 @@ def format_request_details(user, group, label, holding_id, tag): out += f"holding_id: {holding_id}, " if tag: out += f"tag: {tag}, " + if transaction_id: + out += f"transaction_id: {transaction_id}" + if sub_id: + out += f"sub_id: {sub_id}" + if state: + out += f"state: {state}" + if retry_count: + out += f"retry_count: {retry_count}" return out[:-2] def print_list(response: dict, req_details): @@ -160,6 +170,14 @@ def print_list(response: dict, req_details): for h in response['data']['holdings']: print(h) +def print_stat(response: dict, req_details): + """Print out the response from the list command""" + stat_string = "State of transactions(s) for: " + stat_string += req_details + print(stat_string) + for r in response['data']['records']: + print(r) + def print_find(response:dict, req_details): """Print out the response from the find command""" @@ -175,7 +193,7 @@ def list(user, group, label, holding_id, tag): try: response = list_holding(user, group, label, holding_id, tag) req_details = format_request_details( - user, group, label, holding_id, tag + user, group, label=label, holding_id=holding_id, tag=tag ) if response['success'] and len(response['data']['holdings']) > 0: print_list(response, req_details) @@ -201,9 +219,13 @@ def list(user, group, label, holding_id, tag): @click.option("--retry_count", default=None, type=int) def stat(user, group, transaction_id, sub_id, state, retry_count): try: - response = monitor_transactions(user, group, transaction_id, sub_id, state, - retry_count) - print(response) + response = monitor_transactions(user, group, transaction_id, sub_id, + state, retry_count) + req_details = format_request_details( + user, group, transaction_id=transaction_id, sub_id=sub_id, + state=state, retry_count=retry_count + ) + print_stat(response, req_details) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: From 58879069a7be234f61c34fcb2e1123d61f7e8d10 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 15 Nov 2022 13:58:26 +0000 Subject: [PATCH 18/49] Removed some print statements --- nlds_client/clientlib/transactions.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index cf44240..7a72d02 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -63,7 +63,6 @@ def process_transaction_response(response: requests.models.Response, url: str, triggered. :rtype: requests.models.Response object """ - print(response.status_code) # possible responses: 202, 400, 403, 404, 422. try: if (response.status_code == requests.codes.ok or @@ -93,7 +92,6 @@ def process_transaction_response(response: requests.models.Response, url: str, response.status_code ) elif (response.status_code == requests.codes.not_found): # 404 - print(response.content) response_msg = response.json()['detail'] raise RequestError( f"Could not complete the request to the URL: {url} \n" From 4d8713bd8539813013683831298a3aa8f0684b99 Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Wed, 16 Nov 2022 15:54:09 +0000 Subject: [PATCH 19/49] Fix minor typos --- nlds_client/clientlib/transactions.py | 2 +- nlds_client/nlds_client.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 7a72d02..9fd809c 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -485,7 +485,7 @@ def monitor_transactions(user, group, transaction_id, sub_id, state, :param state: applies a state-specific filter to the status request, only sub_records at the given state will be returned. - :type state: string or int, optional + :type state: string, optional :param retry_count: returns sub_records at the given retry_count value :type retry_count: int, optional diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 1a2973a..0a11924 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -153,13 +153,13 @@ def format_request_details(user, group, label=None, holding_id=None, if tag: out += f"tag: {tag}, " if transaction_id: - out += f"transaction_id: {transaction_id}" + out += f"transaction_id: {transaction_id}, " if sub_id: - out += f"sub_id: {sub_id}" + out += f"sub_id: {sub_id}, " if state: - out += f"state: {state}" + out += f"state: {state}, " if retry_count: - out += f"retry_count: {retry_count}" + out += f"retry_count: {retry_count}, " return out[:-2] def print_list(response: dict, req_details): @@ -172,7 +172,7 @@ def print_list(response: dict, req_details): def print_stat(response: dict, req_details): """Print out the response from the list command""" - stat_string = "State of transactions(s) for: " + stat_string = "State of transaction(s) for: " stat_string += req_details print(stat_string) for r in response['data']['records']: @@ -215,7 +215,7 @@ def list(user, group, label, holding_id, tag): @click.option("--group", default=None, type=str) @click.option("--transaction_id", default=None, type=str) @click.option("--sub_id", default=None, type=str) -@click.option("--state", default=None, type=(str, int)) +@click.option("--state", default=None, type=str) @click.option("--retry_count", default=None, type=int) def stat(user, group, transaction_id, sub_id, state, retry_count): try: From 457462c2a6907d852d9e834344654057a4bd71fc Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Thu, 17 Nov 2022 11:43:09 +0000 Subject: [PATCH 20/49] Added find hooks, improved error handling --- nlds_client/clientlib/transactions.py | 109 +++++++++++++++++++++++--- nlds_client/nlds_client.py | 8 +- 2 files changed, 103 insertions(+), 14 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 7a72d02..14727ff 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -38,7 +38,8 @@ def construct_server_url(config: Dict, method=""): return url -def process_transaction_response(response: requests.models.Response, url: str, +def process_transaction_response(response: requests.models.Response, + url: str, config: Dict): """Process the response to raise exceptions for errors or return the response result. @@ -204,8 +205,11 @@ def main_loop(url: str, def put_filelist(filelist: List[str]=[], - user: str=None, group: str=None, - label: str=None, holding_id: int=None, tag: dict=None): + user: str=None, + group: str=None, + label: str=None, + holding_id: int=None, + tag: dict=None): """Make a request to put a list of files into the NLDS. :param filelist: the list of filepaths to put into storage :type filelist: List[string] @@ -290,8 +294,12 @@ def put_filelist(filelist: List[str]=[], def get_filelist(filelist: List[str]=[], - user: str=None, group: str=None, target: str = None, - label: str=None, holding_id: int=None, tag: dict=None) -> Dict: + user: str=None, + group: str=None, + target: str = None, + label: str=None, + holding_id: int=None, + tag: dict=None) -> Dict: """Make a request to get a list of files from the NLDS. :param filelist: the list of filepaths to get from the storage :type filelist: List[string] @@ -397,8 +405,11 @@ def get_filelist(filelist: List[str]=[], return response_dict -def list_holding(user: str, group: str, - label: str, holding_id: int, tag: dict): +def list_holding(user: str, + group: str, + label: str=None, + holding_id: int=None, + tag: dict=None): """Make a request to list the holdings in the NLDS for a user :param user: the username to get the holding(s) for :type user: string @@ -428,7 +439,7 @@ def list_holding(user: str, group: str, config = load_config() user = get_user(config, user) group = get_group(config, group) - url = construct_server_url(config, "holdings") + url = construct_server_url(config, "catalog/list") # build the parameters. holdings->get requires # user: str @@ -462,12 +473,86 @@ def list_holding(user: str, group: str, return response_dict -def find_file(): - pass +def find_file(user: str, + group: str, + label: str=None, + holding_id: int=None, + path: str=None, + tag: dict=None): + """Make a request to find files in the NLDS for a user + :param user: the username to get the holding(s) for + :type user: string + + :param group: the group to get the holding(s) for + :type group: string + + :param label: the label of an existing holding to get the details for + :type label: str, optional + + :param holding_id: the integer id of an existing holding to get the details + :type holding_id: int, optional + + :param tag: a list of key:value pairs to search holdings for - return + holdings with these tags. This will be converted to dictionary before + calling the remote method. + :type tag: dict, optional + + :param path: path to search for, can be a substring, regex or wildcard + :type path: str, optional + + :raises requests.exceptions.ConnectionError: if the server cannot be + reached + + :return: A Dictionary of the response + :rtype: Dict + """ + # get the config, user and group + config = load_config() + user = get_user(config, user) + group = get_group(config, group) + url = construct_server_url(config, "catalog/find") + + # build the parameters. holdings->get requires + # user: str + # group: str + input_params = {"user" : user, + "group" : group} + + # add additional / optional components to input params + if label is not None: + input_params["label"] = label + if tag is not None: + # convert dict to string + tag_str = "" + for key in tag: + tag_str += key+":"+tag[key]+"," + input_params["tag"] = str(tag).replace("'","") + if holding_id is not None: + input_params["holding_id"] = holding_id + if path is not None: + input_params["path"] = path + + print(url, input_params) + response_dict = main_loop( + url=url, + input_params=input_params, + method=requests.get + ) + + if not response_dict: + response_dict = { + "msg" : f"FIND files for user {user} and group {group} failed", + "success" : False + } + return response_dict -def monitor_transactions(user, group, transaction_id, sub_id, state, - retry_count): +def monitor_transactions(user: str, + group: str, + transaction_id: str=None, + sub_id: str=None, + state: str=None, + retry_count: int=None): """Make a request to the monitoring database for a status update of ongoing or finished transactions in the NLDS for, a user/group diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 1a2973a..eb24d86 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -200,6 +200,8 @@ def list(user, group, label, holding_id, tag): else: fail_string = "Failed to list holding with " fail_string += req_details + if 'failure' in response['details']: + fail_string += "\nReason: " + response['details']['failure'] raise click.UsageError(fail_string) except ConnectionError as ce: @@ -241,18 +243,20 @@ def stat(user, group, transaction_id, sub_id, state, retry_count): @click.option("--holding_id", default=None, type=int) @click.option("--path", default=None, type=str) @click.option("--tag", default=None, type=TAG_PARAM_TYPE) -def list(user, group, label, holding_id, tag): +def find(user, group, label, holding_id, path, tag): # try: response = find_file(user, group, label, holding_id, path, tag) req_details = format_request_details( user, group, label, holding_id, tag ) - if response['success'] and len(response['data']['holdings']) > 0: + if response['success'] and len(response['data']['files']) > 0: print_find(response, req_details) else: fail_string = "Failed to list files with " fail_string += req_details + if response['details']['failure']: + fail_string += "\n" + response['details']['failure'] raise click.UsageError(fail_string) except ConnectionError as ce: From 5d4b7fb819ddefa9f2b37d9ea141ebe75590d599 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Wed, 23 Nov 2022 12:27:14 +0000 Subject: [PATCH 21/49] Added meta command --- nlds_client/clientlib/transactions.py | 105 +++++++++++++++++--- nlds_client/nlds_client.py | 138 +++++++++++++++++--------- 2 files changed, 183 insertions(+), 60 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 7ab4596..759c67b 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -119,6 +119,17 @@ def process_transaction_response(response: requests.models.Response, ) +def tag_to_string(tag: dict): + """Convert a dictionary of tags into comma delimited string of key:value + pairs.""" + # convert dict to string + tag_str = "" + for key in tag: + tag_str += key+":"+tag[key]+"," + tag_str = tag_str.replace("'","") + return tag_str + + def main_loop(url: str, input_params: dict={}, body_params: dict={}, @@ -273,7 +284,7 @@ def put_filelist(filelist: List[str]=[], if label is not None: body_params["label"] = label if tag is not None: - body_params["tag"] = tag + body_params["tag"] = tag_to_string(tag) if holding_id is not None: body_params["holding_id"] = holding_id # make the request @@ -384,7 +395,7 @@ def get_filelist(filelist: List[str]=[], if label is not None: body_params["label"] = label if tag is not None: - body_params["tag"] = tag + body_params["tag"] = tag_to_string(tag) if holding_id is not None: body_params["holding_id"] = holding_id # make the request @@ -451,11 +462,7 @@ def list_holding(user: str, if label is not None: input_params["label"] = label if tag is not None: - # convert dict to string - tag_str = "" - for key in tag: - tag_str += key+":"+tag[key]+"," - input_params["tag"] = str(tag).replace("'","") + input_params["tag"] = tag_to_string(tag) if holding_id is not None: input_params["holding_id"] = holding_id @@ -522,17 +529,12 @@ def find_file(user: str, if label is not None: input_params["label"] = label if tag is not None: - # convert dict to string - tag_str = "" - for key in tag: - tag_str += key+":"+tag[key]+"," - input_params["tag"] = str(tag).replace("'","") + input_params["tag"] = tag_to_string(tag) if holding_id is not None: input_params["holding_id"] = holding_id if path is not None: input_params["path"] = path - print(url, input_params) response_dict = main_loop( url=url, input_params=input_params, @@ -618,3 +620,80 @@ def monitor_transactions(user: str, "success": False } return response_dict + + +def change_metadata(user: str, + group: str, + label: str=None, + holding_id: int=None, + tag: dict=None, + new_label: str=None, + new_tag: dict=None): + """Make a request to change the metadata for a NLDS holding for a user + :param user: the username to change the holding(s) for + :type user: string + + :param group: the group to change the holding(s) for + :type group: string + + :param label: the label of an existing holding to change the details for + :type label: str, optional + + :param holding_id: the integer id of an existing holding to change the details + :type holding_id: int, optional + + :param tag: a list of key:value pairs to search holdings for - return + holdings with these tags. This will be converted to dictionary before + calling the remote method. + :type tag: dict, optional + + :param new_label: the new label to change the label to for the holding + :type new_label: str, optional + + :param new_tag: the tag to add / change for the holding + :type new_tag: dict, optional + + :raises requests.exceptions.ConnectionError: if the server cannot be + reached + + :return: A Dictionary of the response + :rtype: Dict + """ + # get the config, user and group + config = load_config() + user = get_user(config, user) + group = get_group(config, group) + url = construct_server_url(config, "catalog/meta") + + # build the parameters. holdings->get requires + # user: str + # group: str + input_params = {"user" : user, + "group" : group} + body_params = {} + # add additional / optional components to input params + if label is not None: + input_params["label"] = label + if tag is not None: + input_params["tag"] = tag_to_string(tag) + if holding_id is not None: + input_params["holding_id"] = holding_id + # new metadata to amend / overwrite / add + if new_label is not None: + body_params["new_label"] = new_label + if new_tag is not None: + body_params["new_tag"] = new_tag + + response_dict = main_loop( + url=url, + input_params=input_params, + body_params=body_params, + method=requests.post # post method as we are changing a resource + ) + + if not response_dict: + response_dict = { + "msg" : f"FIND files for user {user} and group {group} failed", + "success" : False + } + return response_dict \ No newline at end of file diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index f7c6a0a..4061989 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -2,7 +2,8 @@ import click from nlds_client.clientlib.transactions import (get_filelist, put_filelist, list_holding, find_file, - monitor_transactions) + monitor_transactions, + change_metadata) from nlds_client.clientlib.exceptions import ConnectionError, RequestError, \ AuthenticationError from nlds_client.clientlib.config import get_user, get_group, load_config @@ -33,6 +34,61 @@ def convert(self, value, param, ctx): TAG_PARAM_TYPE = TagParamType() +def format_request_details(user, group, label=None, holding_id=None, + tag=None, transaction_id=None, sub_id=None, + state=None, retry_count=None): + config = load_config() + out = "" + user = get_user(config, user) + out += f"user: {user}, " + + group = get_group(config, group) + out += f"group: {group}, " + + if label: + out += f"label: {label}, " + if holding_id: + out += f"holding_id: {holding_id}, " + if tag: + out += f"tag: {tag}, " + if transaction_id: + out += f"transaction_id: {transaction_id}, " + if sub_id: + out += f"sub_id: {sub_id}, " + if state: + out += f"state: {state}, " + if retry_count: + out += f"retry_count: {retry_count}, " + return out[:-2] + + +def print_list(response: dict, req_details): + """Print out the response from the list command""" + list_string = "Listing holding(s) for: " + list_string += req_details + print(list_string) + for h in response['data']['holdings']: + print(h) + + +def print_stat(response: dict, req_details): + """Print out the response from the list command""" + stat_string = "State of transaction(s) for: " + stat_string += req_details + print(stat_string) + for r in response['data']['records']: + print(r) + + +def print_find(response:dict, req_details): + """Print out the response from the find command""" + print(response) + + +def print_meta(response:dict, req_details): + """Print out the response from the meta command""" + print(response) + """Put files command""" @nlds_client.command("put") @@ -135,51 +191,6 @@ def getlist(filelist, user, group, target, label, holding_id, tag): except RequestError as re: raise click.UsageError(re) -def format_request_details(user, group, label=None, holding_id=None, - tag=None, transaction_id=None, sub_id=None, - state=None, retry_count=None): - config = load_config() - out = "" - user = get_user(config, user) - out += f"user: {user}, " - - group = get_group(config, group) - out += f"group: {group}, " - - if label: - out += f"label: {label}, " - if holding_id: - out += f"holding_id: {holding_id}, " - if tag: - out += f"tag: {tag}, " - if transaction_id: - out += f"transaction_id: {transaction_id}, " - if sub_id: - out += f"sub_id: {sub_id}, " - if state: - out += f"state: {state}, " - if retry_count: - out += f"retry_count: {retry_count}, " - return out[:-2] - -def print_list(response: dict, req_details): - """Print out the response from the list command""" - list_string = "Listing holding(s) for: " - list_string += req_details - print(list_string) - for h in response['data']['holdings']: - print(h) - -def print_stat(response: dict, req_details): - """Print out the response from the list command""" - stat_string = "State of transaction(s) for: " - stat_string += req_details - print(stat_string) - for r in response['data']['records']: - print(r) - -def print_find(response:dict, req_details): - """Print out the response from the find command""" """List (holdings) command""" @nlds_client.command("list") @@ -195,7 +206,7 @@ def list(user, group, label, holding_id, tag): req_details = format_request_details( user, group, label=label, holding_id=holding_id, tag=tag ) - if response['success'] and len(response['data']['holdings']) > 0: + if response['success'] and not 'failure' in response['details']: print_list(response, req_details) else: fail_string = "Failed to list holding with " @@ -266,6 +277,39 @@ def find(user, group, label, holding_id, path, tag): except RequestError as re: raise click.UsageError(re) +"""Meta command""" +@nlds_client.command("meta") +@click.option("--user", default=None, type=str) +@click.option("--group", default=None, type=str) +@click.option("--label", default=None, type=str) +@click.option("--holding_id", default=None, type=int) +@click.option("--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("--new_label", default=None, type=str) +@click.option("--new_tag", default=None, type=TAG_PARAM_TYPE) +def meta(user, group, label, holding_id, tag, new_label, new_tag): + # + try: + response = change_metadata(user, group, label, holding_id, tag, + new_label, new_tag) + req_details = format_request_details( + user, group, label, holding_id, tag + ) + if response['success'] > 0: + print_meta(response, req_details) + else: + fail_string = "Failed to list files with " + fail_string += req_details + if response['details']['failure']: + fail_string += "\n" + response['details']['failure'] + raise click.UsageError(fail_string) + + except ConnectionError as ce: + raise click.UsageError(ce) + except AuthenticationError as ae: + raise click.UsageError(ae) + except RequestError as re: + raise click.UsageError(re) + def main(): nlds_client(prog_name="nlds") From e1161a51cb9a3a062ccb6275a613eceeb69c658d Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Wed, 23 Nov 2022 12:34:08 +0000 Subject: [PATCH 22/49] Add short options for flags in stat command --- nlds_client/nlds_client.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 4061989..96be8c6 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -224,12 +224,12 @@ def list(user, group, label, holding_id, tag): """Stat (monitoring) command""" @nlds_client.command("stat") -@click.option("--user", default=None, type=str) -@click.option("--group", default=None, type=str) -@click.option("--transaction_id", default=None, type=str) -@click.option("--sub_id", default=None, type=str) -@click.option("--state", default=None, type=str) -@click.option("--retry_count", default=None, type=int) +@click.option("-u", "--user", default=None, type=str) +@click.option("-g", "--group", default=None, type=str) +@click.option("-t", "--transaction_id", default=None, type=str) +@click.option("-s", "--sub_id", default=None, type=str) +@click.option("-S", "--state", default=None, type=str) +@click.option("-r", "--retry_count", default=None, type=int) def stat(user, group, transaction_id, sub_id, state, retry_count): try: response = monitor_transactions(user, group, transaction_id, sub_id, From 1386f4804126394f61d223a78ec782c088e1e7ac Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Thu, 24 Nov 2022 15:58:21 +0000 Subject: [PATCH 23/49] Nicely formatted file command output --- nlds_client/clientlib/transactions.py | 32 +++- nlds_client/nlds_client.py | 218 ++++++++++++++++++++++---- 2 files changed, 212 insertions(+), 38 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 759c67b..cc9283f 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -297,10 +297,15 @@ def put_filelist(filelist: List[str]=[], if not response_dict: # If we get to this point then the transaction could not be processed - response_dict = {'uuid' : str(transaction_id), - 'msg' : f'PUT transaction with id {transaction_id} failed', - 'success' : False - } + response_dict = { + 'uuid' : str(transaction_id), + 'msg' : f'PUT transaction with id {transaction_id} failed', + 'success' : False + } + # mark as failed in RPC call + elif "failure" in response_dict["details"]: + response_dict["success"] = False + return response_dict @@ -413,6 +418,10 @@ def get_filelist(filelist: List[str]=[], "msg" : f"GET transaction with id {transaction_id} failed", "success" : False } + # mark as failed in RPC call + elif "failure" in response_dict["details"]: + response_dict["success"] = False + return response_dict @@ -477,6 +486,10 @@ def list_holding(user: str, "msg" : f"LIST holdings for user {user} and group {group} failed", "success" : False } + # mark as failed in RPC call + elif "failure" in response_dict["details"]: + response_dict["success"] = False + return response_dict @@ -546,6 +559,10 @@ def find_file(user: str, "msg" : f"FIND files for user {user} and group {group} failed", "success" : False } + # mark as failed in RPC call + elif "failure" in response_dict["details"]: + response_dict["success"] = False + return response_dict @@ -619,6 +636,10 @@ def monitor_transactions(user: str, "msg": f"STAT transaction for user {user} and group {group} failed", "success": False } + # mark as failed in RPC call + elif "failure" in response_dict["details"]: + response_dict["success"] = False + return response_dict @@ -696,4 +717,7 @@ def change_metadata(user: str, "msg" : f"FIND files for user {user} and group {group} failed", "success" : False } + # mark as failed in RPC call + elif "failure" in response_dict["details"]: + response_dict["success"] = False return response_dict \ No newline at end of file diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 4061989..46fe4fc 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -8,11 +8,12 @@ AuthenticationError from nlds_client.clientlib.config import get_user, get_group, load_config +json=False + @click.group() def nlds_client(): pass - """Custom class for tags in the format key:value (i.e. using the colon as separator between key and value.""" @@ -34,41 +35,89 @@ def convert(self, value, param, ctx): TAG_PARAM_TYPE = TagParamType() + +def integer_permissions_to_string(intperm): + octal = oct(intperm)[2:] + result = "" + value_letters = [(4,"r"),(2,"w"),(1,"x")] + # Iterate over each of the digits in octal + for digit in [int(n) for n in str(octal)]: + # Check for each of the permissions values + for value, letter in value_letters: + if digit >= value: + result += letter + digit -= value + else: + result += '-' + return result + + +def pretty_size(size): + '''Returns file size in human readable format''' + + suffixes = [("B", 1), + ("K", 1000), + ("M", 1000000), + ("G", 1000000000), + ("T", 1000000000000)] + level_up_factor = 2000.0 + for suf, multipler in suffixes: + if float(size) / multipler > level_up_factor: + continue + else: + return round(size / float(multipler), 2).__str__() + suf + return round(size / float(multipler), 2).__str__() + suf + + def format_request_details(user, group, label=None, holding_id=None, tag=None, transaction_id=None, sub_id=None, state=None, retry_count=None): config = load_config() out = "" user = get_user(config, user) - out += f"user: {user}, " + out += f"user:{user}, " group = get_group(config, group) - out += f"group: {group}, " + out += f"group:{group}, " if label: - out += f"label: {label}, " + out += f"label:{label}, " if holding_id: - out += f"holding_id: {holding_id}, " + out += f"holding_id:{holding_id}, " if tag: - out += f"tag: {tag}, " + out += f"tag:{tag}, " if transaction_id: - out += f"transaction_id: {transaction_id}, " + out += f"transaction_id:{transaction_id}, " if sub_id: - out += f"sub_id: {sub_id}, " + out += f"sub_id:{sub_id}, " if state: - out += f"state: {state}, " + out += f"state:{state}, " if retry_count: - out += f"retry_count: {retry_count}, " + out += f"retry_count:{retry_count}, " return out[:-2] def print_list(response: dict, req_details): """Print out the response from the list command""" - list_string = "Listing holding(s) for: " + n_holdings = len(response['data']['holdings']) + list_string = "Listing holding" + if n_holdings > 1: + list_string += "s" + list_string += " for: " list_string += req_details - print(list_string) - for h in response['data']['holdings']: - print(h) + click.echo(list_string) + if n_holdings == 1: + h = response['data']['holdings'][0] + click.echo(f"{'':<4}{'id':<16}: {h['id']}") + click.echo(f"{'':<4}{'label':<16}: {h['label']}") + click.echo(f"{'':<4}{'user':<16}: {h['user']}") + click.echo(f"{'':<4}{'group':<16}: {h['group']}") + else: + click.echo(f"{'':<4}{'id':<6}{'label':<16}{'user':<16}{'group':<16}") + for h in response['data']['holdings']: + click.echo( + f"{'':<4}{h['id']:<6}{h['label']:<16}{h['user']:<16}{h['group']:<16}" + ) def print_stat(response: dict, req_details): @@ -80,9 +129,78 @@ def print_stat(response: dict, req_details): print(r) +def __count_files(response:dict): + """Count the number of files returned from a find query""" + n_files = 0 + for h in response['data']['holdings']: + holding = response['data']['holdings'][h] + for t in holding['transactions']: + transaction = holding['transactions'][t] + n_files += len(transaction['filelist']) + return n_files + + +def print_single_file(response): + """Print (full) details of one file""" + # NRM - note: still using loops over dictionary keys as its + # 1. easier than trying to just use the first key + # 2. a bit more robust - in case more than one file matches, for example, in + # separate holdings + for hkey in response['data']['holdings']: + h = response['data']['holdings'][hkey] + for tkey in h['transactions']: + t = h['transactions'][tkey] + time = t['ingest_time'].replace("T", " ") + for f in t['filelist']: + click.echo(f"{'':<4}{'path':<16}: {f['original_path']}") + click.echo(f"{'':<4}{'type':<16}: {f['path_type']}") + if (f['link_path']): + click.echo(f"{'':<4}{'link path':<16}: {f['link_path']}") + size = pretty_size(f['size']) + click.echo(f"{'':<4}{'size':<16}: {size}") + click.echo(f"{'':<4}{'user uid':<16}: {f['user']}") + click.echo(f"{'':<4}{'group gid':<16}: {f['group']}") + click.echo( + f"{'':<4}{'permissions':<16}: " + f"{integer_permissions_to_string(f['permissions'])}" + ) + click.echo(f"{'':<4}{'ingest time':<16}: {time}") + # locations + stls = " " + for s in f['locations']: + stls += s['storage_type']+", " + + click.echo(f"{'':<4}{'storage location':<16}:{stls[0:-2]}") + + +def print_multi_file(response): + click.echo(f"{'':<4}{'h-id':<6}{'h-label':<16}{'path':<64}{'size':<8}{'time':<12}") + for hkey in response['data']['holdings']: + h = response['data']['holdings'][hkey] + for tkey in h['transactions']: + t = h['transactions'][tkey] + time = t['ingest_time'].replace("T", " ") + for f in t['filelist']: + size = pretty_size(f['size']) + click.echo(f"{'':4}{h['holding_id']:<6}{h['label']:<16}" + f"{f['original_path']:<64}{size:<8}" + f"{time:<12}") + def print_find(response:dict, req_details): """Print out the response from the find command""" - print(response) + n_holdings = len(response['data']['holdings']) + list_string = "Listing files for holding" + if n_holdings > 1: + list_string += "s" + list_string += " for: " + list_string += req_details + click.echo(list_string) + # get total number of files + n_files = __count_files(response) + if (n_files == 1): + print_single_file(response) + else: + print_multi_file(response) def print_meta(response:dict, req_details): @@ -97,12 +215,14 @@ def print_meta(response:dict, req_details): @click.option("--label", default=None, type=str) @click.option("--holding_id", default=None, type=int) @click.option("--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("--json", default=False, type=bool) @click.argument("filepath", type=str) -def put(filepath, user, group, label, holding_id, tag): +def put(filepath, user, group, label, holding_id, tag, json): try: response = put_filelist([filepath], user, group, label, holding_id, tag) - print(response) + if json: + print(response) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -119,12 +239,14 @@ def put(filepath, user, group, label, holding_id, tag): @click.option("--label", default=None, type=str) @click.option("--holding_id", default=None, type=int) @click.option("--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("--json", default=False, type=bool) @click.argument("filepath", type=str) -def get(filepath, user, group, target, label, holding_id, tag): +def get(filepath, user, group, target, label, holding_id, tag, json): try: response = get_filelist([filepath], user, group, target, label, holding_id, tag) - print(response) + if json: + print(response) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -141,7 +263,8 @@ def get(filepath, user, group, target, label, holding_id, tag): @click.option("--label", default=None, type=str) @click.option("--holding_id", default=None, type=int) @click.option("--tag", default=None, type=TAG_PARAM_TYPE) -def putlist(filelist, user, group, label, holding_id, tag): +@click.option("--json", default=False, type=bool) +def putlist(filelist, user, group, label, holding_id, tag, json): # read the filelist from the file try: fh = open(filelist) @@ -153,7 +276,9 @@ def putlist(filelist, user, group, label, holding_id, tag): try: response = put_filelist(files, user, group, label, holding_id, tag) - print(response) + if json: + print(response) + except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -170,8 +295,9 @@ def putlist(filelist, user, group, label, holding_id, tag): @click.option("--label", default=None, type=str) @click.option("--holding_id", default=None, type=int) @click.option("--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("--json", default=False, type=bool) @click.argument("filelist", type=str) -def getlist(filelist, user, group, target, label, holding_id, tag): +def getlist(filelist, user, group, target, label, holding_id, tag, json): # read the filelist from the file try: fh = open(filelist) @@ -182,8 +308,9 @@ def getlist(filelist, user, group, target, label, holding_id, tag): try: response = get_filelist(files, user, group, - target, holding_id, tag) - print(response) + target, label, holding_id, tag) + if json: + print(response) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -199,15 +326,19 @@ def getlist(filelist, user, group, target, label, holding_id, tag): @click.option("--label", default=None, type=str) @click.option("--holding_id", default=None, type=int) @click.option("--tag", default=None, type=TAG_PARAM_TYPE) -def list(user, group, label, holding_id, tag): +@click.option("--json", default=False, is_flag=True) +def list(user, group, label, holding_id, tag, json): # try: response = list_holding(user, group, label, holding_id, tag) req_details = format_request_details( user, group, label=label, holding_id=holding_id, tag=tag ) - if response['success'] and not 'failure' in response['details']: - print_list(response, req_details) + if response['success']: + if json: + print(response) + else: + print_list(response, req_details) else: fail_string = "Failed to list holding with " fail_string += req_details @@ -230,7 +361,8 @@ def list(user, group, label, holding_id, tag): @click.option("--sub_id", default=None, type=str) @click.option("--state", default=None, type=str) @click.option("--retry_count", default=None, type=int) -def stat(user, group, transaction_id, sub_id, state, retry_count): +@click.option("--json", default=False, type=bool, is_flag=True) +def stat(user, group, transaction_id, sub_id, state, retry_count, json): try: response = monitor_transactions(user, group, transaction_id, sub_id, state, retry_count) @@ -238,7 +370,17 @@ def stat(user, group, transaction_id, sub_id, state, retry_count): user, group, transaction_id=transaction_id, sub_id=sub_id, state=state, retry_count=retry_count ) - print_stat(response, req_details) + if response['success']: + if json: + print(response) + else: + print_stat(response, req_details) + else: + fail_string = "Failed to get status of transaction(s) with " + fail_string += req_details + if 'failure' in response['details']: + fail_string += "\nReason: " + response['details']['failure'] + raise click.UsageError(fail_string) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -254,15 +396,19 @@ def stat(user, group, transaction_id, sub_id, state, retry_count): @click.option("--holding_id", default=None, type=int) @click.option("--path", default=None, type=str) @click.option("--tag", default=None, type=TAG_PARAM_TYPE) -def find(user, group, label, holding_id, path, tag): +@click.option("--json", default=False, type=bool, is_flag=True) +def find(user, group, label, holding_id, path, tag, json): # try: response = find_file(user, group, label, holding_id, path, tag) req_details = format_request_details( user, group, label, holding_id, tag ) - if response['success'] and len(response['data']['files']) > 0: - print_find(response, req_details) + if response['success']: + if json: + print(response) + else: + print_find(response, req_details) else: fail_string = "Failed to list files with " fail_string += req_details @@ -286,7 +432,8 @@ def find(user, group, label, holding_id, path, tag): @click.option("--tag", default=None, type=TAG_PARAM_TYPE) @click.option("--new_label", default=None, type=str) @click.option("--new_tag", default=None, type=TAG_PARAM_TYPE) -def meta(user, group, label, holding_id, tag, new_label, new_tag): +@click.option("--json", default=False, type=bool, is_flag=True) +def meta(user, group, label, holding_id, tag, new_label, new_tag, json): # try: response = change_metadata(user, group, label, holding_id, tag, @@ -295,7 +442,10 @@ def meta(user, group, label, holding_id, tag, new_label, new_tag): user, group, label, holding_id, tag ) if response['success'] > 0: - print_meta(response, req_details) + if json: + print_meta(response, req_details) + else: + print(response) else: fail_string = "Failed to list files with " fail_string += req_details From 079ec5045ff2ecf326ad991bf10b07f232d24afe Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Thu, 1 Dec 2022 15:00:10 +0000 Subject: [PATCH 24/49] Add api-action flag into stat command --- nlds_client/clientlib/transactions.py | 8 ++++++++ nlds_client/nlds_client.py | 14 +++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index cc9283f..781beeb 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -569,6 +569,7 @@ def find_file(user: str, def monitor_transactions(user: str, group: str, transaction_id: str=None, + api_action: str=None, sub_id: str=None, state: str=None, retry_count: int=None): @@ -583,6 +584,11 @@ def monitor_transactions(user: str, :param transaction_id: a specific transaction_id to get the status of :type transaction_id: string, optional + + :param api_action: applies an api-action-specific filter to the status + request, only transaction_records of the given api_action will be + returned. + :type api_action: string, optional :param sub_id: a specific sub_id (of a sub_record) to get the status of :type sub_id: string, optional @@ -617,6 +623,8 @@ def monitor_transactions(user: str, # add additional / optional components to input params if transaction_id is not None: input_params["transaction_id"] = transaction_id + if api_action is not None: + input_params["api_action"] = api_action if sub_id is not None: input_params["sub_id"] = sub_id if state is not None: diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index b7a294c..b8297e2 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -71,7 +71,7 @@ def pretty_size(size): def format_request_details(user, group, label=None, holding_id=None, tag=None, transaction_id=None, sub_id=None, - state=None, retry_count=None): + state=None, retry_count=None, api_action=None): config = load_config() out = "" user = get_user(config, user) @@ -94,6 +94,8 @@ def format_request_details(user, group, label=None, holding_id=None, out += f"state:{state}, " if retry_count: out += f"retry_count:{retry_count}, " + if api_action: + out += f"api-action:{api_action}, " return out[:-2] @@ -358,17 +360,19 @@ def list(user, group, label, holding_id, tag, json): @click.option("-u", "--user", default=None, type=str) @click.option("-g", "--group", default=None, type=str) @click.option("-t", "--transaction_id", default=None, type=str) +@click.option("-a", "--api_action", default=None, type=str) @click.option("-s", "--sub_id", default=None, type=str) @click.option("-S", "--state", default=None, type=str) @click.option("-r", "--retry_count", default=None, type=int) @click.option("-j", "--json", default=False, type=bool, is_flag=True) -def stat(user, group, transaction_id, sub_id, state, retry_count, json): +def stat(user, group, transaction_id, api_action, sub_id, state, retry_count, + json): try: - response = monitor_transactions(user, group, transaction_id, sub_id, - state, retry_count) + response = monitor_transactions(user, group, transaction_id, api_action, + sub_id, state, retry_count) req_details = format_request_details( user, group, transaction_id=transaction_id, sub_id=sub_id, - state=state, retry_count=retry_count + state=state, retry_count=retry_count, api_action=api_action, ) if response['success']: if json: From 4e325161b1b1e4834763ee69d08b3aa883e8ac43 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Thu, 1 Dec 2022 15:49:45 +0000 Subject: [PATCH 25/49] A small amount of refactoring for the monitor - more to come --- nlds_client/clientlib/transactions.py | 25 ++++++++++++++++++------- nlds_client/nlds_client.py | 2 ++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index cc9283f..1f004f8 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -303,7 +303,7 @@ def put_filelist(filelist: List[str]=[], 'success' : False } # mark as failed in RPC call - elif "failure" in response_dict["details"]: + elif "details" in response_dict and "failure" in response_dict["details"]: response_dict["success"] = False return response_dict @@ -410,7 +410,6 @@ def get_filelist(filelist: List[str]=[], body_params=body_params, method=requests.put ) - if not response_dict: # If we get to this point then the transaction could not be processed response_dict = { @@ -419,7 +418,7 @@ def get_filelist(filelist: List[str]=[], "success" : False } # mark as failed in RPC call - elif "failure" in response_dict["details"]: + elif "details" in response_dict and "failure" in response_dict["details"]: response_dict["success"] = False return response_dict @@ -487,7 +486,7 @@ def list_holding(user: str, "success" : False } # mark as failed in RPC call - elif "failure" in response_dict["details"]: + elif "details" in response_dict and "failure" in response_dict["details"]: response_dict["success"] = False return response_dict @@ -560,7 +559,7 @@ def find_file(user: str, "success" : False } # mark as failed in RPC call - elif "failure" in response_dict["details"]: + elif "details" in response_dict and "failure" in response_dict["details"]: response_dict["success"] = False return response_dict @@ -637,12 +636,24 @@ def monitor_transactions(user: str, "success": False } # mark as failed in RPC call - elif "failure" in response_dict["details"]: + elif "details" in response_dict and "failure" in response_dict["details"]: response_dict["success"] = False return response_dict +def process_monitor_transactions(transactions: dict): + """Process the transactions into a more convienent form by processing the + sub-transactions and determining if the overall transaction is complete. + Should we move this code to the server? + Transaction dictionary looks like this: + { + 'transaction_record': + { + 'id': 4, + 'transaction_id': '4028fd97-7cbb-49a8-9dc2-9e241fe60ece', 'user': 'nrmassey', 'group': 'cedaproc', 'api_action': 'getlist', 'creation_time': '2022-11-28T15:51:18'}, 'sub_record': {'id': 4, 'sub_id': '9ef906fd-f187-4ebd-86ee-71869fd7d752', 'state': 'COMPLETE', 'retry_count': 0, 'last_updated': '2022-11-28T15:51:19'}, 'failed_files': []} + """ + def change_metadata(user: str, group: str, label: str=None, @@ -718,6 +729,6 @@ def change_metadata(user: str, "success" : False } # mark as failed in RPC call - elif "failure" in response_dict["details"]: + elif "details" in response_dict and "failure" in response_dict["details"]: response_dict["success"] = False return response_dict \ No newline at end of file diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index b7a294c..9f5e574 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -3,6 +3,7 @@ from nlds_client.clientlib.transactions import (get_filelist, put_filelist, list_holding, find_file, monitor_transactions, + process_monitor_transactions, change_metadata) from nlds_client.clientlib.exceptions import ConnectionError, RequestError, \ AuthenticationError @@ -120,6 +121,7 @@ def print_list(response: dict, req_details): ) + def print_stat(response: dict, req_details): """Print out the response from the list command""" stat_string = "State of transaction(s) for: " From c4557194e828c55a7571cad182f063f98c46d617 Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Fri, 2 Dec 2022 13:59:04 +0000 Subject: [PATCH 26/49] Remove old unittest-style test --- tests/test_authentication.py | 73 ------------------------------------ 1 file changed, 73 deletions(-) delete mode 100644 tests/test_authentication.py diff --git a/tests/test_authentication.py b/tests/test_authentication.py deleted file mode 100644 index 8036991..0000000 --- a/tests/test_authentication.py +++ /dev/null @@ -1,73 +0,0 @@ -import os -import unittest -from unittest.mock import patch -import nlds_client.clientlib.authentication as clau -import json -import requests -import nlds_client.clientlib.exceptions as exc - -TEST_USERNAME = 'testuser' -TEST_PASSWORD = 'testpswd' -TEST_TOKEN_TEXT = '{"access_token": "pojp", "expires_in": 36000, "token_type": "Bearer", "scope": "test scope", "refresh_token": "popojp"}' - -class MockResponse: - def __init__(self, status_code, text=TEST_TOKEN_TEXT): - self.status_code = status_code - self.text = text - - -def mock_requests_post(*args, **kwargs): - if kwargs['data']['username'] == TEST_USERNAME and kwargs['data']['password'] == TEST_PASSWORD: - return MockResponse(requests.codes.ok) - elif kwargs['data']['username'] == TEST_USERNAME and kwargs['data']['password'] != TEST_PASSWORD: - return MockResponse(requests.codes.unauthorized) - elif kwargs['data']['username'] != TEST_USERNAME: - return MockResponse(requests.codes.forbidden) - - -class TestAuthentication(unittest.TestCase): - def setUp(self) -> None: - template_filename = os.path.join(os.path.dirname(__file__), '../templates/nlds-config.j2') - fh = open(template_filename) - self.config = json.load(fh) - - def test_response_ok(self): - response = MockResponse(requests.codes.ok) - self.assertEqual(clau.process_fetch_oauth2_token_response(self.config, response), - response) - - def test_response_bad_request(self): - response = MockResponse(requests.codes.bad_request) - with self.assertRaises(exc.RequestError): - clau.process_fetch_oauth2_token_response(self.config, response) - - def test_response_unauthorised(self): - response = MockResponse(requests.codes.unauthorized) - with self.assertRaises(exc.AuthenticationError): - clau.process_fetch_oauth2_token_response(self.config, response) - - def test_response_forbidden(self): - response = MockResponse(requests.codes.forbidden) - with self.assertRaises(exc.AuthenticationError): - clau.process_fetch_oauth2_token_response(self.config, response) - - def test_response_not_found(self): - response = MockResponse(requests.codes.not_found) - with self.assertRaises(exc.RequestError): - clau.process_fetch_oauth2_token_response(self.config, response) - - def test_response_undefined(self): - response = MockResponse(None) - with self.assertRaises(exc.RequestError): - clau.process_fetch_oauth2_token_response(self.config, response) - - @patch('requests.post', side_effect=mock_requests_post) - def test_fetch(self, mock_post): - self.assertEqual(clau.fetch_oauth2_token(self.config, TEST_USERNAME, TEST_PASSWORD), json.loads(TEST_TOKEN_TEXT)) - with self.assertRaises(exc.AuthenticationError): - clau.fetch_oauth2_token(self.config, TEST_USERNAME, '') - with self.assertRaises(exc.AuthenticationError): - clau.fetch_oauth2_token(self.config, '', TEST_PASSWORD) - -if __name__ == '__main__': - unittest.main() From 2b9c52f8b85a15b846598afddb8ec5ea00802feb Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Fri, 2 Dec 2022 14:01:03 +0000 Subject: [PATCH 27/49] Reorder imports properly --- tests/test_oauth.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_oauth.py b/tests/test_oauth.py index d8ae985..86f89c8 100644 --- a/tests/test_oauth.py +++ b/tests/test_oauth.py @@ -1,12 +1,14 @@ import os -import pytest -import requests import json +import dotenv +import copy +import requests + +import pytest + import nlds_client.clientlib.authentication as clau import nlds_client.clientlib.config as clcnf import nlds_client.clientlib.exceptions as exc -import dotenv -import copy # Load environment files from .env file dotenv.load_dotenv() From ac74597cf962c7030306d4704325136691e4764f Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Fri, 2 Dec 2022 16:15:25 +0000 Subject: [PATCH 28/49] Add auto-testing github workflow --- .github/.workflows/testing.yml | 41 ++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 .github/.workflows/testing.yml diff --git a/.github/.workflows/testing.yml b/.github/.workflows/testing.yml new file mode 100644 index 0000000..5834634 --- /dev/null +++ b/.github/.workflows/testing.yml @@ -0,0 +1,41 @@ +# This workflow will install Python dependencies, run tests and lint with a variety of Python versions +# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions + +name: auto-testing + +on: + push: + branches: [ "main", "development" ] + pull_request: + branches: [ "main", "development" ] + +jobs: + build: + + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.8", "3.9", "3.10"] + + steps: + - uses: actions/checkout@v3 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v3 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install flake8 pytest + if [ -f requirements.txt ]; then pip install -r requirements.txt; fi + python -m pip install -e . + - name: Lint with flake8 + run: | + # stop the build if there are Python syntax errors or undefined names + flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics + # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide + flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics + - name: Test with pytest + run: | + pytest From f8a7276b16a1359594dc495677a1a82846096196 Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Fri, 2 Dec 2022 16:21:14 +0000 Subject: [PATCH 29/49] Fix typo in workflow directory structure --- .github/{.workflows => workflows}/testing.yml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .github/{.workflows => workflows}/testing.yml (100%) diff --git a/.github/.workflows/testing.yml b/.github/workflows/testing.yml similarity index 100% rename from .github/.workflows/testing.yml rename to .github/workflows/testing.yml From 700a931364f400fdeaf80160dd8388c2891ab93d Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Tue, 6 Dec 2022 09:53:40 +0000 Subject: [PATCH 30/49] Skip integration tests that require dotenv when running CI testing on github --- tests/test_oauth.py | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/tests/test_oauth.py b/tests/test_oauth.py index 86f89c8..c19fbba 100644 --- a/tests/test_oauth.py +++ b/tests/test_oauth.py @@ -1,6 +1,5 @@ import os import json -import dotenv import copy import requests @@ -10,15 +9,26 @@ import nlds_client.clientlib.config as clcnf import nlds_client.clientlib.exceptions as exc -# Load environment files from .env file -dotenv.load_dotenv() - -TEST_USERNAME = os.environ['USERNAME'] -TEST_PASSWORD = os.environ['PASSWORD'] -TEST_TOKEN_TEXT = os.environ['TOKEN_TEXT'] -TEST_CONFIG_FILE = os.environ.get('CONFIG_FILE') EDGE_VALUES = ['', None, ' ',] +@pytest.fixture +def login_details(): + dotenv = pytest.importorskip( + "dotenv", + reason="This is an integration unit test that requires a local " + "configuration environment file.", + ) + # Load environment files from .env file + dotenv.load_dotenv() + + return { + 'username': os.environ['USERNAME'], + 'password': os.environ['PASSWORD'], + 'token_text': os.environ['TOKEN_TEXT'], + 'config_file': os.environ.get('CONFIG_FILE') + } + + @pytest.fixture def basic_config(): # Read the template config file into a json object @@ -27,7 +37,8 @@ def basic_config(): return json.load(fh) @pytest.fixture -def functional_config(): +def functional_config(login_details): + TEST_CONFIG_FILE = login_details['config_file'] # Read the users actual config file so test calls can be made to the API if TEST_CONFIG_FILE is None: return clcnf.load_config() @@ -37,11 +48,14 @@ def functional_config(): class MockResponse: # A mock response object for checking error handling capabilities - def __init__(self, status_code=200, text=TEST_TOKEN_TEXT): + def __init__(self, status_code=200, text='TEST_TOKEN_TEXT'): self.status_code = status_code self.text = text def test_get_username_password(monkeypatch, basic_config): + TEST_USERNAME = 'test' + TEST_PASSWORD = 'test_pwd' + # Mock the username and password getting by overriding the input and getpass functions monkeypatch.setattr('builtins.input', lambda _: TEST_USERNAME) monkeypatch.setattr('getpass.getpass', lambda _: TEST_PASSWORD) @@ -66,7 +80,10 @@ def test_process_fetch_oauth2_token_response(basic_config, status_code): @pytest.mark.parametrize("test_value", EDGE_VALUES) -def test_fetch_oauth_token(functional_config, test_value): +def test_fetch_oauth_token(functional_config, test_value, login_details): + TEST_USERNAME = login_details['username'] + TEST_PASSWORD = login_details['password'] + # Test correct username and password token = clau.fetch_oauth2_token(functional_config, TEST_USERNAME, TEST_PASSWORD) assert isinstance(token, dict) @@ -84,7 +101,10 @@ def test_fetch_oauth_token(functional_config, test_value): clau.fetch_oauth2_token(functional_config, test_value, test_value) @pytest.mark.parametrize("test_value", EDGE_VALUES) -def test_fetch_oauth_config(monkeypatch, functional_config, test_value): +def test_fetch_oauth_config(monkeypatch, functional_config, test_value, + login_details): + TEST_USERNAME = login_details['username'] + TEST_PASSWORD = login_details['password'] monkeypatch.setattr(clau, 'save_token', None) modified_config = copy.deepcopy(functional_config) From a721acbb00c1034506fd0b02eb81f578b72526d8 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 6 Dec 2022 16:54:12 +0000 Subject: [PATCH 31/49] User interactivity --- nlds_client/clientlib/transactions.py | 80 ++++++++++++++++++++++++--- nlds_client/nlds_client.py | 32 ++++++++--- 2 files changed, 95 insertions(+), 17 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 8813394..420a721 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -4,6 +4,7 @@ import urllib.parse import os from pathlib import Path +from datetime import datetime from typing import List, Dict @@ -650,17 +651,80 @@ def monitor_transactions(user: str, return response_dict -def process_monitor_transactions(transactions: dict): - """Process the transactions into a more convienent form by processing the - sub-transactions and determining if the overall transaction is complete. - Should we move this code to the server? +def get_transaction_state(transaction: dict): + """Get the overall state of a transaction in a more convienent form by + querying the sub-transactions and determining if the overall transaction + is complete. Transaction dictionary looks like this: { - 'transaction_record': - { - 'id': 4, - 'transaction_id': '4028fd97-7cbb-49a8-9dc2-9e241fe60ece', 'user': 'nrmassey', 'group': 'cedaproc', 'api_action': 'getlist', 'creation_time': '2022-11-28T15:51:18'}, 'sub_record': {'id': 4, 'sub_id': '9ef906fd-f187-4ebd-86ee-71869fd7d752', 'state': 'COMPLETE', 'retry_count': 0, 'last_updated': '2022-11-28T15:51:19'}, 'failed_files': []} + 'id': 2, + 'transaction_id': 'a06ec7b3-e83c-4ac7-97d8-2545a0b8d317', + 'user': 'nrmassey', + 'group': 'cedaproc', + 'api_action': 'getlist', + 'creation_time': '2022-12-06T15:45:43', + 'sub_records': [ + { + 'id': 2, + 'sub_id': '007075b2-8c79-4cfa-a1e5-0aaa65892454', + 'state': 'COMPLETE', + 'retry_count': 0, + 'last_updated': '2022-12-06T15:45:44', + 'failed_files': [] + } + ] + } + + possible values of state are: + INITIALISING = -1 + ROUTING = 0 + SPLITTING = 1 + INDEXING = 2 + TRANSFER_PUTTING = 3 + CATALOG_PUTTING = 4 + CATALOG_GETTING = 5 + TRANSFER_GETTING = 6 + COMPLETE = 8 + FAILED = 9 + The overall state is the minimum of these """ + state_mapping = { + "INITIALISING" : -1, + "ROUTING" : 0, + "SPLITTING" : 1, + "INDEXING" : 2, + "TRANSFER_PUTTING" : 3, + "CATALOG_PUTTING" : 4, + "CATALOG_GETTING" : 5, + "TRANSFER_GETTING" : 6, + "COMPLETE" : 8, + "FAILED" : 9, + } + state_mapping_reverse = { + -1 : "INITIALISING", + 0 : "ROUTING", + 1 : "SPLITTING", + 2 : "INDEXING", + 3 : "TRANSFER_PUTTING" , + 4 : "CATALOG_PUTTING", + 5 : "CATALOG_GETTING", + 6 : "TRANSFER_GETTING", + 8 : "COMPLETE", + 9 : "FAILED", + } + + min_state = 100 + min_time = datetime(1970,1,1) + for sr in transaction["sub_records"]: + sr_state = sr["state"] + d = datetime.fromisoformat(sr["last_updated"]) + if(d > min_time): + min_time = d + if state_mapping[sr_state] < min_state: + min_state = state_mapping[sr_state] + + return state_mapping_reverse[min_state], min_time + def change_metadata(user: str, group: str, diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index fd9a5f3..1e3e842 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -3,7 +3,7 @@ from nlds_client.clientlib.transactions import (get_filelist, put_filelist, list_holding, find_file, monitor_transactions, - process_monitor_transactions, + get_transaction_state, change_metadata) from nlds_client.clientlib.exceptions import ConnectionError, RequestError, \ AuthenticationError @@ -113,24 +113,29 @@ def print_list(response: dict, req_details): h = response['data']['holdings'][0] click.echo(f"{'':<4}{'id':<16}: {h['id']}") click.echo(f"{'':<4}{'label':<16}: {h['label']}") - click.echo(f"{'':<4}{'user':<16}: {h['user']}") - click.echo(f"{'':<4}{'group':<16}: {h['group']}") + # click.echo(f"{'':<4}{'user':<16}: {h['user']}") + # click.echo(f"{'':<4}{'group':<16}: {h['group']}") else: - click.echo(f"{'':<4}{'id':<6}{'label':<16}{'user':<16}{'group':<16}") + # click.echo(f"{'':<4}{'id':<6}{'label':<16}{'user':<16}{'group':<16}") + click.echo(f"{'':<4}{'id':<6}{'label':<16}") for h in response['data']['holdings']: click.echo( - f"{'':<4}{h['id']:<6}{h['label']:<16}{h['user']:<16}{h['group']:<16}" + # f"{'':<4}{h['id']:<6}{h['label']:<16}{h['user']:<16}{h['group']:<16}" + f"{'':<4}{h['id']:<6}{h['label']:<16}" ) - def print_stat(response: dict, req_details): """Print out the response from the list command""" stat_string = "State of transaction(s) for: " stat_string += req_details - print(stat_string) - for r in response['data']['records']: - print(r) + click.echo(stat_string) + # print format heading + click.echo(f"{'':<4}{'id':<6}{'action':<12}{'transaction id':<48}{'state':<12}{'last update':<20}") + for tr in response['data']['records']: + state, time = get_transaction_state(tr) + time = time.isoformat().replace("T"," ") + click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}{tr['transaction_id']:<48}{state:<12}{time:<20}") def __count_files(response:dict): @@ -190,6 +195,7 @@ def print_multi_file(response): f"{f['original_path']:<64}{size:<8}" f"{time:<12}") + def print_find(response:dict, req_details): """Print out the response from the find command""" n_holdings = len(response['data']['holdings']) @@ -227,6 +233,8 @@ def put(filepath, user, group, label, holding_id, tag, json): label, holding_id, tag) if json: print(response) + else: + print(response['msg'].strip('\n')) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -251,6 +259,8 @@ def get(filepath, user, group, target, label, holding_id, tag, json): label, holding_id, tag) if json: print(response) + else: + print(response['msg'].strip('\n')) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -282,6 +292,8 @@ def putlist(filelist, user, group, label, holding_id, tag, json): label, holding_id, tag) if json: print(response) + else: + print(response['msg'].strip('\n')) except ConnectionError as ce: raise click.UsageError(ce) @@ -315,6 +327,8 @@ def getlist(filelist, user, group, target, label, holding_id, tag, json): target, label, holding_id, tag) if json: print(response) + else: + print(response['msg'].strip('\n')) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: From 317221a639ec406f4625c0f1807187a4e58bc166 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Mon, 12 Dec 2022 16:09:54 +0000 Subject: [PATCH 32/49] Changes ot monitor / stat UI --- nlds_client/clientlib/exceptions.py | 3 + nlds_client/clientlib/transactions.py | 11 +- nlds_client/nlds_client.py | 229 ++++++++++++++++---------- 3 files changed, 157 insertions(+), 86 deletions(-) diff --git a/nlds_client/clientlib/exceptions.py b/nlds_client/clientlib/exceptions.py index c713caa..6243ec8 100644 --- a/nlds_client/clientlib/exceptions.py +++ b/nlds_client/clientlib/exceptions.py @@ -23,3 +23,6 @@ class UsageError(Exception): class ConfigError(Exception): pass + +class StateError(Exception): + pass \ No newline at end of file diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 420a721..bf55202 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -568,10 +568,11 @@ def find_file(user: str, def monitor_transactions(user: str, group: str, + idd: int=None, transaction_id: str=None, api_action: str=None, - sub_id: str=None, state: str=None, + sub_id: str=None, retry_count: int=None): """Make a request to the monitoring database for a status update of ongoing or finished transactions in the NLDS for, a user/group @@ -582,6 +583,9 @@ def monitor_transactions(user: str, :param group: the group to get the transaction state(s) for :type group: string + :param idd: the numeric id (primary key) of the transaction + :type idd: int + :param transaction_id: a specific transaction_id to get the status of :type transaction_id: string, optional @@ -621,6 +625,8 @@ def monitor_transactions(user: str, "group" : group} # add additional / optional components to input params + if idd is not None: + input_params["id"] = idd if transaction_id is not None: input_params["transaction_id"] = transaction_id if api_action is not None: @@ -723,6 +729,9 @@ def get_transaction_state(transaction: dict): if state_mapping[sr_state] < min_state: min_state = state_mapping[sr_state] + if min_state == 100: + return None, None + return state_mapping_reverse[min_state], min_time diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 1e3e842..6dab636 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -71,7 +71,7 @@ def pretty_size(size): def format_request_details(user, group, label=None, holding_id=None, - tag=None, transaction_id=None, sub_id=None, + tag=None, id=None, transaction_id=None, sub_id=None, state=None, retry_count=None, api_action=None): config = load_config() out = "" @@ -83,6 +83,8 @@ def format_request_details(user, group, label=None, holding_id=None, if label: out += f"label:{label}, " + if id: + out += f"id:{id}, " if holding_id: out += f"holding_id:{holding_id}, " if tag: @@ -106,7 +108,7 @@ def print_list(response: dict, req_details): list_string = "Listing holding" if n_holdings > 1: list_string += "s" - list_string += " for: " + list_string += " for " list_string += req_details click.echo(list_string) if n_holdings == 1: @@ -124,18 +126,63 @@ def print_list(response: dict, req_details): f"{'':<4}{h['id']:<6}{h['label']:<16}" ) - -def print_stat(response: dict, req_details): - """Print out the response from the list command""" - stat_string = "State of transaction(s) for: " +def print_single_stat(response: dict, req_details): + """Print a single status in more detail, with a list of failed files if + necessary""" + stat_string = "State of transaction: " + click.echo(stat_string) + # still looping over the keys, just in case more than one state returned + for tr in response['data']['records']: + state, _ = get_transaction_state(tr) + if state == None: + continue + click.echo(f"{'':<4}{'id':<15}: {tr['id']}") + click.echo(f"{'':<4}{'user':<15}: {tr['user']}") + click.echo(f"{'':<4}{'group':<15}: {tr['group']}") + click.echo(f"{'':<4}{'action':<15}: {tr['api_action']}") + click.echo(f"{'':<4}{'transaction id':<15}: {tr['transaction_id']}") + click.echo(f"{'':<4}{'creation time':<15}: {(tr['creation_time']).replace('T',' ')}") + click.echo(f"{'':<4}{'state':<15}: {state}") + click.echo(f"{'':<4}{'sub records':<15}->") + for sr in tr['sub_records']: + click.echo(f"{'':4}{'+':<4} {'id':<13}: {sr['id']}") + click.echo(f"{'':<9}{'sub_id':<13}: {sr['sub_id']}") + click.echo(f"{'':<9}{'state':<13}: {sr['state']}") + click.echo(f"{'':<9}{'retries':<13}: {sr['retry_count']}") + click.echo(f"{'':<9}{'last update':<13}: {(sr['last_updated']).replace('T',' ')}") + + if len(sr['failed_files']) > 0: + click.echo(f"{'':<9}{'failed files':<13}->") + for ff in sr['failed_files']: + click.echo(f"{'':<9}{'+':<4} {'filepath':<8} : {ff['filepath']}") + click.echo(f"{'':<9}{'':>4} {'reason':<8} : {ff['reason']}") + + +def print_multi_stat(response: dict, req_details): + """Print a multi-line set of status""" + stat_string = "State of transactions for " stat_string += req_details click.echo(stat_string) - # print format heading - click.echo(f"{'':<4}{'id':<6}{'action':<12}{'transaction id':<48}{'state':<12}{'last update':<20}") + click.echo(f"{'':<4}{'id':<6}{'action':<12}{'transaction id':<48}" + f"{'state':<12}{'last update':<20}") for tr in response['data']['records']: state, time = get_transaction_state(tr) + if state == None: + continue time = time.isoformat().replace("T"," ") - click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}{tr['transaction_id']:<48}{state:<12}{time:<20}") + click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}" + f"{tr['transaction_id']:<48}{state:<12}{time:<20}") + + +def print_stat(response: dict, req_details): + """Print out the response from the list command""" + L = len(response['data']['records']) + if L == 0: + click.echo("No transactions found") + elif L == 1: + print_single_stat(response, req_details) + else: + print_multi_stat(response, req_details) def __count_files(response:dict): @@ -202,7 +249,7 @@ def print_find(response:dict, req_details): list_string = "Listing files for holding" if n_holdings > 1: list_string += "s" - list_string += " for: " + list_string += " for " list_string += req_details click.echo(list_string) # get total number of files @@ -215,26 +262,35 @@ def print_find(response:dict, req_details): def print_meta(response:dict, req_details): """Print out the response from the meta command""" - print(response) + meta_string = "Changing metadata for holding for " + meta_string += req_details + click.echo(meta_string) + for h in response['data']['holdings']: + click.echo("Old metadata: ") + click.echo(f"{'':<4}{'Label:':<8}{h['old_meta']['label']}") + click.echo(f"{'':<4}{'Tags:':<8}{h['old_meta']['tags']}") + click.echo("New metadata: ") + click.echo(f"{'':<4}{'Label:':<8}{h['new_meta']['label']}") + click.echo(f"{'':<4}{'Tags:':<8}{h['new_meta']['tags']}") """Put files command""" @nlds_client.command("put") -@click.option("--user", default=None, type=str) -@click.option("--group", default=None, type=str) -@click.option("--label", default=None, type=str) -@click.option("--holding_id", default=None, type=int) -@click.option("--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("--json", default=False, type=bool) +@click.option("-u", "--user", default=None, type=str) +@click.option("-g", "--group", default=None, type=str) +@click.option("-l", "--label", default=None, type=str) +@click.option("-i", "--holding_id", default=None, type=int) +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("-j", "--json", default=False, type=bool) @click.argument("filepath", type=str) def put(filepath, user, group, label, holding_id, tag, json): try: response = put_filelist([filepath], user, group, label, holding_id, tag) if json: - print(response) + click.echo(response) else: - print(response['msg'].strip('\n')) + click.echo(response['msg'].strip('\n')) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -245,22 +301,22 @@ def put(filepath, user, group, label, holding_id, tag, json): """Get files command""" @nlds_client.command("get") -@click.option("--user", default=None, type=str) -@click.option("--group", default=None, type=str) -@click.option("--target", default=None, type=click.Path()) -@click.option("--label", default=None, type=str) -@click.option("--holding_id", default=None, type=int) -@click.option("--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("--json", default=False, type=bool) +@click.option("-u", "--user", default=None, type=str) +@click.option("-g", "--group", default=None, type=str) +@click.option("-t", "--target", default=None, type=click.Path()) +@click.option("-l", "--label", default=None, type=str) +@click.option("-i", "--holding_id", default=None, type=int) +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("-j", "--json", default=False, type=bool) @click.argument("filepath", type=str) def get(filepath, user, group, target, label, holding_id, tag, json): try: response = get_filelist([filepath], user, group, target, label, holding_id, tag) if json: - print(response) + click.echo(response) else: - print(response['msg'].strip('\n')) + click.echo(response['msg'].strip('\n')) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -272,12 +328,12 @@ def get(filepath, user, group, target, label, holding_id, tag, json): """Put filelist command""" @nlds_client.command("putlist") @click.argument("filelist", type=str) -@click.option("--user", default=None, type=str) -@click.option("--group", default=None, type=str) -@click.option("--label", default=None, type=str) -@click.option("--holding_id", default=None, type=int) -@click.option("--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("--json", default=False, type=bool) +@click.option("-u", "--user", default=None, type=str) +@click.option("-g", "--group", default=None, type=str) +@click.option("-l", "--label", default=None, type=str) +@click.option("-i", "--holding_id", default=None, type=int) +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("-j", "--json", default=False, type=bool) def putlist(filelist, user, group, label, holding_id, tag, json): # read the filelist from the file try: @@ -291,9 +347,9 @@ def putlist(filelist, user, group, label, holding_id, tag, json): response = put_filelist(files, user, group, label, holding_id, tag) if json: - print(response) + click.echo(response) else: - print(response['msg'].strip('\n')) + click.echo(response['msg'].strip('\n')) except ConnectionError as ce: raise click.UsageError(ce) @@ -305,13 +361,13 @@ def putlist(filelist, user, group, label, holding_id, tag, json): """Get filelist command""" @nlds_client.command("getlist") -@click.option("--user", default=None, type=str) -@click.option("--group", default=None, type=str) -@click.option("--target", default=None, type=click.Path(exists=True)) -@click.option("--label", default=None, type=str) -@click.option("--holding_id", default=None, type=int) -@click.option("--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("--json", default=False, type=bool) +@click.option("-u", "--user", default=None, type=str) +@click.option("-g", "--group", default=None, type=str) +@click.option("-t", "--target", default=None, type=click.Path(exists=True)) +@click.option("-l", "--label", default=None, type=str) +@click.option("-i", "--holding_id", default=None, type=int) +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("-j", "--json", default=False, type=bool) @click.argument("filelist", type=str) def getlist(filelist, user, group, target, label, holding_id, tag, json): # read the filelist from the file @@ -326,9 +382,9 @@ def getlist(filelist, user, group, target, label, holding_id, tag, json): response = get_filelist(files, user, group, target, label, holding_id, tag) if json: - print(response) + click.echo(response) else: - print(response['msg'].strip('\n')) + click.echo(response['msg'].strip('\n')) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -339,22 +395,24 @@ def getlist(filelist, user, group, target, label, holding_id, tag, json): """List (holdings) command""" @nlds_client.command("list") -@click.option("--user", default=None, type=str) -@click.option("--group", default=None, type=str) -@click.option("--label", default=None, type=str) -@click.option("--holding_id", default=None, type=int) -@click.option("--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("--json", default=False, is_flag=True) +@click.option("-u", "--user", default=None, type=str) +@click.option("-g", "--group", default=None, type=str) +@click.option("-l", "--label", default=None, type=str) +@click.option("-i", "--holding_id", default=None, type=int) +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("-j", "--json", default=False, is_flag=True) def list(user, group, label, holding_id, tag, json): # try: - response = list_holding(user, group, label, holding_id, tag) + response = list_holding( + user, group, label, holding_id, tag + ) req_details = format_request_details( - user, group, label=label, holding_id=holding_id, tag=tag - ) + user, group, label=label, holding_id=holding_id, tag=tag + ) if response['success']: if json: - print(response) + click.echo(response) else: print_list(response, req_details) else: @@ -371,28 +429,28 @@ def list(user, group, label, holding_id, tag, json): except RequestError as re: raise click.UsageError(re) + """Stat (monitoring) command""" @nlds_client.command("stat") @click.option("-u", "--user", default=None, type=str) @click.option("-g", "--group", default=None, type=str) -@click.option("-t", "--transaction_id", default=None, type=str) +@click.option("-i", "--id", default=None, type=int) +@click.option("-T", "--transaction_id", default=None, type=str) @click.option("-a", "--api_action", default=None, type=str) -@click.option("-s", "--sub_id", default=None, type=str) -@click.option("-S", "--state", default=None, type=str) -@click.option("-r", "--retry_count", default=None, type=int) +@click.option("-s", "--state", default=None, type=str) @click.option("-j", "--json", default=False, type=bool, is_flag=True) -def stat(user, group, transaction_id, api_action, sub_id, state, retry_count, - json): +def stat(user, group, id, transaction_id, api_action, state, json): try: - response = monitor_transactions(user, group, transaction_id, api_action, - sub_id, state, retry_count) + response = monitor_transactions( + user, group, id, transaction_id, api_action, state, + ) req_details = format_request_details( - user, group, transaction_id=transaction_id, sub_id=sub_id, - state=state, retry_count=retry_count, api_action=api_action, + user, group, id=id, transaction_id=transaction_id, state=state, + api_action=api_action, ) if response['success']: if json: - print(response) + click.echo(response) else: print_stat(response, req_details) else: @@ -408,15 +466,16 @@ def stat(user, group, transaction_id, api_action, sub_id, state, retry_count, except RequestError as re: raise click.UsageError(re) + """Find (files) command""" @nlds_client.command("find") -@click.option("--user", default=None, type=str) -@click.option("--group", default=None, type=str) -@click.option("--label", default=None, type=str) -@click.option("--holding_id", default=None, type=int) -@click.option("--path", default=None, type=str) -@click.option("--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("--json", default=False, type=bool, is_flag=True) +@click.option("-u", "--user", default=None, type=str) +@click.option("-g", "--group", default=None, type=str) +@click.option("-l", "--label", default=None, type=str) +@click.option("-i", "--holding_id", default=None, type=int) +@click.option("-p", "--path", default=None, type=str) +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("-j", "--json", default=False, type=bool, is_flag=True) def find(user, group, label, holding_id, path, tag, json): # try: @@ -426,7 +485,7 @@ def find(user, group, label, holding_id, path, tag, json): ) if response['success']: if json: - print(response) + click.echo(response) else: print_find(response, req_details) else: @@ -445,14 +504,14 @@ def find(user, group, label, holding_id, path, tag, json): """Meta command""" @nlds_client.command("meta") -@click.option("--user", default=None, type=str) -@click.option("--group", default=None, type=str) -@click.option("--label", default=None, type=str) -@click.option("--holding_id", default=None, type=int) -@click.option("--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("--new_label", default=None, type=str) -@click.option("--new_tag", default=None, type=TAG_PARAM_TYPE) -@click.option("--json", default=False, type=bool, is_flag=True) +@click.option("-u", "--user", default=None, type=str) +@click.option("-g", "--group", default=None, type=str) +@click.option("-l", "--label", default=None, type=str) +@click.option("-i", "--holding_id", default=None, type=int) +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) +@click.option("-L", "--new_label", default=None, type=str) +@click.option("-T", "--new_tag", default=None, type=TAG_PARAM_TYPE) +@click.option("-j", "--json", default=False, type=bool, is_flag=True) def meta(user, group, label, holding_id, tag, new_label, new_tag, json): # try: @@ -463,11 +522,11 @@ def meta(user, group, label, holding_id, tag, new_label, new_tag, json): ) if response['success'] > 0: if json: - print_meta(response, req_details) + click.echo(response) else: - print(response) + print_meta(response, req_details) else: - fail_string = "Failed to list files with " + fail_string = "Failed to change metadata on holding with " fail_string += req_details if response['details']['failure']: fail_string += "\n" + response['details']['failure'] From 9c0ded55c5ea511f0ab4966165994ca29ee427f8 Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Thu, 5 Jan 2023 12:55:38 +0000 Subject: [PATCH 33/49] Add label to outputs of stat command --- nlds_client/nlds_client.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 6dab636..f2186d7 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -141,6 +141,8 @@ def print_single_stat(response: dict, req_details): click.echo(f"{'':<4}{'group':<15}: {tr['group']}") click.echo(f"{'':<4}{'action':<15}: {tr['api_action']}") click.echo(f"{'':<4}{'transaction id':<15}: {tr['transaction_id']}") + if 'label' in tr: + click.echo(f"{'':<4}{'label':<15}: {tr['label']}") click.echo(f"{'':<4}{'creation time':<15}: {(tr['creation_time']).replace('T',' ')}") click.echo(f"{'':<4}{'state':<15}: {state}") click.echo(f"{'':<4}{'sub records':<15}->") @@ -170,8 +172,13 @@ def print_multi_stat(response: dict, req_details): if state == None: continue time = time.isoformat().replace("T"," ") - click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}" - f"{tr['transaction_id']:<48}{state:<12}{time:<20}") + if 'label' in tr: + click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}" + f"{tr['transaction_id']:<48}{tr['label']:<48}" + f"{state:<12}{time:<20}") + else: + click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}" + f"{tr['transaction_id']:<48}{state:<12}{time:<20}") def print_stat(response: dict, req_details): From 6ceac33aca2d009daf17a1838a212735cfcb9cec Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Mon, 9 Jan 2023 15:38:43 +0000 Subject: [PATCH 34/49] Added help text --- nlds_client/nlds_client.py | 227 +++++++++++++++++++++++++++---------- 1 file changed, 166 insertions(+), 61 deletions(-) diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 6dab636..a807999 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -115,15 +115,16 @@ def print_list(response: dict, req_details): h = response['data']['holdings'][0] click.echo(f"{'':<4}{'id':<16}: {h['id']}") click.echo(f"{'':<4}{'label':<16}: {h['label']}") + click.echo(f"{'':<4}{'ingest time':<16}: {h['date'].replace('T',' ')}") # click.echo(f"{'':<4}{'user':<16}: {h['user']}") # click.echo(f"{'':<4}{'group':<16}: {h['group']}") else: # click.echo(f"{'':<4}{'id':<6}{'label':<16}{'user':<16}{'group':<16}") - click.echo(f"{'':<4}{'id':<6}{'label':<16}") + click.echo(f"{'':<4}{'id':<6}{'label':<16}{'ingest time':<32}") for h in response['data']['holdings']: click.echo( # f"{'':<4}{h['id']:<6}{h['label']:<16}{h['user']:<16}{h['group']:<16}" - f"{'':<4}{h['id']:<6}{h['label']:<16}" + f"{'':<4}{h['id']:<6}{h['label']:<16}{h['date'].replace('T',' '):<32}" ) def print_single_stat(response: dict, req_details): @@ -163,15 +164,15 @@ def print_multi_stat(response: dict, req_details): stat_string = "State of transactions for " stat_string += req_details click.echo(stat_string) - click.echo(f"{'':<4}{'id':<6}{'action':<12}{'transaction id':<48}" - f"{'state':<12}{'last update':<20}") + click.echo(f"{'':<4}{'id':<6}{'action':<12}{'transaction id':<40}" + f"{'state':<18}{'last update':<20}") for tr in response['data']['records']: state, time = get_transaction_state(tr) if state == None: continue time = time.isoformat().replace("T"," ") click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}" - f"{tr['transaction_id']:<48}{state:<12}{time:<20}") + f"{tr['transaction_id']:<40}{state:<18}{time:<20}") def print_stat(response: dict, req_details): @@ -275,7 +276,29 @@ def print_meta(response:dict, req_details): """Put files command""" -@nlds_client.command("put") +@nlds_client.command("put", help="Put a single file.") +@click.option("-u", "--user", default=None, type=str, + help="The username to put the file for.") +@click.option("-g", "--group", default=None, type=str, + help="The group to put the file for.") +@click.option("-l", "--label", default=None, type=str, + help="The label of the holding to put the file into. If the " + "holding already exists then the file will be added to it. If the " + "holding does not exist then it will be created with the label. " + "If this option is omitted then a new holding with a random label " + "will be created.") +@click.option("-i", "--holding_id", default=None, type=int, + help="The numeric id of an existing holding to put the file into.") +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, + help="The tags of a holding to put the file into. If a holding " + "with the tags already exists then the file will the added to " + "that holding. If the holding does not exist then it will be " + "created with the tags and either a random label or a named label," + "if the label parameter is also supplied. The set of tags must " + "guarantee uniqueness of the holding.") +@click.option("-j", "--json", default=False, type=bool, + help="Output the result as JSON.") + @click.option("-u", "--user", default=None, type=str) @click.option("-g", "--group", default=None, type=str) @click.option("-l", "--label", default=None, type=str) @@ -299,15 +322,30 @@ def put(filepath, user, group, label, holding_id, tag, json): raise click.UsageError(re) +user_help_text = (" If no user or group is given then these values will " + "default to the user:default_user and user:default values " + "in the ~/.nlds-config file.") + """Get files command""" -@nlds_client.command("get") -@click.option("-u", "--user", default=None, type=str) -@click.option("-g", "--group", default=None, type=str) -@click.option("-t", "--target", default=None, type=click.Path()) -@click.option("-l", "--label", default=None, type=str) -@click.option("-i", "--holding_id", default=None, type=int) -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("-j", "--json", default=False, type=bool) +@nlds_client.command("get", + help=f"Get a single file.{user_help_text}") + +@click.option("-u", "--user", default=None, type=str, + help="The username to get a file for.") +@click.option("-g", "--group", default=None, type=str, + help="The group to get a file for.") +@click.option("-t", "--target", default=None, type=click.Path(exists=True), + help="The target path for the retrieved file. Default is to " + "retrieve the file to its original path.") +@click.option("-l", "--label", default=None, type=str, + help="The label of the holding to retrieve the file from. This " + "can be a regular expression (regex).") +@click.option("-i", "--holding_id", default=None, type=int, + help="The id of the holding to retrieve the file from.") +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, + help="The tag(s) of the holding to retrieve the file from.") +@click.option("-j", "--json", default=False, type=bool, + help="Output the result as JSON.") @click.argument("filepath", type=str) def get(filepath, user, group, target, label, holding_id, tag, json): try: @@ -326,14 +364,30 @@ def get(filepath, user, group, target, label, holding_id, tag, json): """Put filelist command""" -@nlds_client.command("putlist") +@nlds_client.command("putlist", + help=f"Put a number of files specified in a list.{user_help_text}") @click.argument("filelist", type=str) -@click.option("-u", "--user", default=None, type=str) -@click.option("-g", "--group", default=None, type=str) -@click.option("-l", "--label", default=None, type=str) -@click.option("-i", "--holding_id", default=None, type=int) -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("-j", "--json", default=False, type=bool) +@click.option("-u", "--user", default=None, type=str, + help="The username to put files for.") +@click.option("-g", "--group", default=None, type=str, + help="The group to put files for.") +@click.option("-l", "--label", default=None, type=str, + help="The label of the holding to put files into. If the holding " + "already exists then the files will be added to it. If the " + "holding does not exist then it will be created with the label. " + "If this option is omitted then a new holding with a random label " + "will be created.") +@click.option("-i", "--holding_id", default=None, type=int, + help="The numeric id of an existing holding to put files into.") +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, + help="The tags of a holding to put files into. If a holding " + "with the tags already exists then the files will the added to " + "that holding. If the holding does not exist then it will be " + "created with the tags and either a random label or a named label," + "if the label parameter is also supplied. The set of tags must " + "guarantee uniqueness of the holding.") +@click.option("-j", "--json", default=False, type=bool, + help="Output the result as JSON.") def putlist(filelist, user, group, label, holding_id, tag, json): # read the filelist from the file try: @@ -360,14 +414,24 @@ def putlist(filelist, user, group, label, holding_id, tag, json): """Get filelist command""" -@nlds_client.command("getlist") -@click.option("-u", "--user", default=None, type=str) -@click.option("-g", "--group", default=None, type=str) -@click.option("-t", "--target", default=None, type=click.Path(exists=True)) -@click.option("-l", "--label", default=None, type=str) -@click.option("-i", "--holding_id", default=None, type=int) -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("-j", "--json", default=False, type=bool) +@nlds_client.command("getlist", + help=f"Get a number of files specified in a list.{user_help_text}") +@click.option("-u", "--user", default=None, type=str, + help="The username to get files for.") +@click.option("-g", "--group", default=None, type=str, + help="The group to get files for.") +@click.option("-t", "--target", default=None, type=click.Path(exists=True), + help="The target path for the retrieved files. Default is to " + "retrieve files to their original path.") +@click.option("-l", "--label", default=None, type=str, + help="The label of the holding(s) to retrieve files from. This " + "can be a regular expression (regex).") +@click.option("-i", "--holding_id", default=None, type=int, + help="The id of the holding to retrieve files from.") +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, + help="The tag(s) of the holding(s) to retrieve files from.") +@click.option("-j", "--json", default=False, type=bool, + help="Output the result as JSON.") @click.argument("filelist", type=str) def getlist(filelist, user, group, target, label, holding_id, tag, json): # read the filelist from the file @@ -394,13 +458,21 @@ def getlist(filelist, user, group, target, label, holding_id, tag, json): """List (holdings) command""" -@nlds_client.command("list") -@click.option("-u", "--user", default=None, type=str) -@click.option("-g", "--group", default=None, type=str) -@click.option("-l", "--label", default=None, type=str) -@click.option("-i", "--holding_id", default=None, type=int) -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("-j", "--json", default=False, is_flag=True) +@nlds_client.command("list", + help=f"List holdings.{user_help_text}") +@click.option("-u", "--user", default=None, type=str, + help="The username to list holdings for.") +@click.option("-g", "--group", default=None, type=str, + help="The group to list holdings for.") +@click.option("-l", "--label", default=None, type=str, + help="The label of the holding(s) to list. This can be a regular" + "expression (regex).") +@click.option("-i", "--holding_id", default=None, type=int, + help="The numeric id of the holding to list.") +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, + help="The tag(s) of the holding(s) to list.") +@click.option("-j", "--json", default=False, is_flag=True, + help="Output the result as JSON.") def list(user, group, label, holding_id, tag, json): # try: @@ -431,14 +503,27 @@ def list(user, group, label, holding_id, tag, json): """Stat (monitoring) command""" -@nlds_client.command("stat") -@click.option("-u", "--user", default=None, type=str) -@click.option("-g", "--group", default=None, type=str) -@click.option("-i", "--id", default=None, type=int) -@click.option("-T", "--transaction_id", default=None, type=str) -@click.option("-a", "--api_action", default=None, type=str) -@click.option("-s", "--state", default=None, type=str) -@click.option("-j", "--json", default=False, type=bool, is_flag=True) +@nlds_client.command("stat", + help=f"List transactions.{user_help_text}") +@click.option("-u", "--user", default=None, type=str, + help="The username to list transactions for.") +@click.option("-g", "--group", default=None, type=str, + help="The group to list transactions for.") +@click.option("-i", "--id", default=None, type=int, + help="The numeric id of the transaction to list.") +@click.option("-T", "--transaction_id", default=None, type=str, + help="The UUID transaction id of the transaction to list.") +@click.option("-a", "--api_action", default=None, type=str, + help="The api action of the transactions to list. Options: get | " + "put | getlist | putlist") +@click.option("-s", "--state", default=None, type=str, + help="\bThe state of the transactions to list. Options: " + "INITIALISING | ROUTING | SPLITTING | INDEXING | " + "TRANSFER_PUTTING | CATALOG_PUTTING | CATALOG_GETTING | " + "TRANSFER_GETTING | COMPLETE | FAILED") +@click.option("-j", "--json", default=False, type=bool, is_flag=True, + help="Output the result as JSON.") + def stat(user, group, id, transaction_id, api_action, state, json): try: response = monitor_transactions( @@ -468,14 +553,24 @@ def stat(user, group, id, transaction_id, api_action, state, json): """Find (files) command""" -@nlds_client.command("find") -@click.option("-u", "--user", default=None, type=str) -@click.option("-g", "--group", default=None, type=str) -@click.option("-l", "--label", default=None, type=str) -@click.option("-i", "--holding_id", default=None, type=int) -@click.option("-p", "--path", default=None, type=str) -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("-j", "--json", default=False, type=bool, is_flag=True) +@nlds_client.command("find", + help=f"Find and list files.{user_help_text}") +@click.option("-u", "--user", default=None, type=str, + help="The username to find files for.") +@click.option("-g", "--group", default=None, type=str, + help="The group to find files for.") +@click.option("-l", "--label", default=None, type=str, + help="The label of the holding which the files belong to. This " + "can be a regular expression (regex).") +@click.option("-i", "--holding_id", default=None, type=int, + help="The numeric id of the holding which the files belong to.") +@click.option("-p", "--path", default=None, type=str, + help="The path of the files to find. This can be a regular " + "expression (regex)") +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, + help="The tag(s) of the holding(s) to find files within.") +@click.option("-j", "--json", default=False, type=bool, is_flag=True, + help="Output the result as JSON.") def find(user, group, label, holding_id, path, tag, json): # try: @@ -503,15 +598,25 @@ def find(user, group, label, holding_id, path, tag, json): raise click.UsageError(re) """Meta command""" -@nlds_client.command("meta") -@click.option("-u", "--user", default=None, type=str) -@click.option("-g", "--group", default=None, type=str) -@click.option("-l", "--label", default=None, type=str) -@click.option("-i", "--holding_id", default=None, type=int) -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("-L", "--new_label", default=None, type=str) -@click.option("-T", "--new_tag", default=None, type=TAG_PARAM_TYPE) -@click.option("-j", "--json", default=False, type=bool, is_flag=True) +@nlds_client.command("meta", + help=f"Alter metadata for a holding.{user_help_text}") +@click.option("-u", "--user", default=None, type=str, + help="The username to use when changing metadata for a holding.") +@click.option("-g", "--group", default=None, type=str, + help="The group to use when changing metadata for a holding.") +@click.option("-l", "--label", default=None, type=str, + help="The label of the holding to change metadata for. This can " + "be a regular expression (regex)") +@click.option("-i", "--holding_id", default=None, type=int, + help="The numeric id of the holding to change metadata for.") +@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, + help="The tag(s) of the holding(s) to change metadata for.") +@click.option("-L", "--new_label", default=None, type=str, + help="The new label for the holding.") +@click.option("-T", "--new_tag", default=None, type=TAG_PARAM_TYPE, + help="The new tag(s) for the holding.") +@click.option("-j", "--json", default=False, type=bool, is_flag=True, + help="Output the result as JSON.") def meta(user, group, label, holding_id, tag, new_label, new_tag, json): # try: From 9e3a2a54fdb1719ef7d09cfd4542751d6a7c13ed Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Wed, 11 Jan 2023 12:23:18 +0000 Subject: [PATCH 35/49] Added transaction id output when listing single holding, fixed label when doing stat --- nlds_client/nlds_client.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index fc196a5..49557df 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -118,6 +118,11 @@ def print_list(response: dict, req_details): click.echo(f"{'':<4}{'ingest time':<16}: {h['date'].replace('T',' ')}") # click.echo(f"{'':<4}{'user':<16}: {h['user']}") # click.echo(f"{'':<4}{'group':<16}: {h['group']}") + if 'transactions' in h: + trans_str = "" + for t in h['transactions']: + trans_str += t + f"\n{'':<22}" + click.echo(f"{'':<4}{'transaction id':<16}: {trans_str[:-23]}") else: # click.echo(f"{'':<4}{'id':<6}{'label':<16}{'user':<16}{'group':<16}") click.echo(f"{'':<4}{'id':<6}{'label':<16}{'ingest time':<32}") @@ -127,6 +132,7 @@ def print_list(response: dict, req_details): f"{'':<4}{h['id']:<6}{h['label']:<16}{h['date'].replace('T',' '):<32}" ) + def print_single_stat(response: dict, req_details): """Print a single status in more detail, with a list of failed files if necessary""" @@ -167,19 +173,19 @@ def print_multi_stat(response: dict, req_details): stat_string += req_details click.echo(stat_string) click.echo(f"{'':<4}{'id':<6}{'action':<12}{'transaction id':<40}" - f"{'state':<18}{'last update':<20}") + f"{'label':<16}{'state':<18}{'last update':<20}") for tr in response['data']['records']: state, time = get_transaction_state(tr) if state == None: continue time = time.isoformat().replace("T"," ") if 'label' in tr: - click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}" - f"{tr['transaction_id']:<48}{tr['label']:<48}" - f"{state:<12}{time:<20}") + label = tr['label'] else: - click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}" - f"{tr['transaction_id']:<48}{state:<12}{time:<20}") + label = "" + click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}" + f"{tr['transaction_id']:<40}{tr['label']:<16}" + f"{state:<12}{time:<20}") def print_stat(response: dict, req_details): From b8043fd097f1d855e07fa75daa6738cfed5a949c Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 17 Jan 2023 15:15:20 +0000 Subject: [PATCH 36/49] Job label functionality --- nlds_client/clientlib/transactions.py | 25 ++++++++++++ nlds_client/nlds_client.py | 56 ++++++++++++++++----------- 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index bf55202..5fcd75d 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -219,6 +219,7 @@ def main_loop(url: str, def put_filelist(filelist: List[str]=[], user: str=None, group: str=None, + job_label: str=None, label: str=None, holding_id: int=None, tag: dict=None): @@ -232,6 +233,9 @@ def put_filelist(filelist: List[str]=[], :param group: the group to put the files :type group: string + :param job_label: an optional label for the transaction to aid user queries + :type job_label: string, optional + :param label: the label of an existing holding that files are to be added to :type label: str, optional @@ -281,6 +285,14 @@ def put_filelist(filelist: List[str]=[], } body_params = {"filelist" : filelist} + # add optional job_label. If None then use first 8 characters of UUID + if job_label is None: + if label is None: + input_params["job_label"] = str(transaction_id)[0:8] + else: + input_params["job_label"] = label + else: + input_params["job_label"] = job_label # add optional components to body: label, tags, holding_id if label is not None: body_params["label"] = label @@ -314,6 +326,7 @@ def get_filelist(filelist: List[str]=[], user: str=None, group: str=None, target: str = None, + job_label: str = None, label: str=None, holding_id: int=None, tag: dict=None) -> Dict: @@ -330,6 +343,9 @@ def get_filelist(filelist: List[str]=[], :param target: the location to write the retrieved files to :type target: string, optional + :param job_label: an optional label for the transaction to aid user queries + :type job_label: string, optional + :param label: the label of an existing holding that files are to be retrieved from :type label: str, optional @@ -386,6 +402,7 @@ def get_filelist(filelist: List[str]=[], # access_key : str # secret_key : str # tenancy : str (optional) + # job_label : str (optional) # target : str (optional - defaults to cwd) # and the filelist in the body input_params = {"transaction_id" : transaction_id, @@ -397,6 +414,14 @@ def get_filelist(filelist: List[str]=[], "target": target, } body_params = {"filelist" : filelist} + # add optional job_label. If None then use first 8 characters of UUID + if job_label is None: + if label is None: + input_params["job_label"] = str(transaction_id)[0:8] + else: + input_params["job_label"] = label + else: + input_params["job_label"] = job_label # add optional components to body: label, tags, holding_id if label is not None: body_params["label"] = label diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 49557df..32abe5d 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -172,7 +172,7 @@ def print_multi_stat(response: dict, req_details): stat_string = "State of transactions for " stat_string += req_details click.echo(stat_string) - click.echo(f"{'':<4}{'id':<6}{'action':<12}{'transaction id':<40}" + click.echo(f"{'':<4}{'id':<6}{'action':<16}{'job label':<16}" f"{'label':<16}{'state':<18}{'last update':<20}") for tr in response['data']['records']: state, time = get_transaction_state(tr) @@ -183,9 +183,13 @@ def print_multi_stat(response: dict, req_details): label = tr['label'] else: label = "" - click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<12}" - f"{tr['transaction_id']:<40}{tr['label']:<16}" - f"{state:<12}{time:<20}") + if 'job_label' in tr and tr['job_label']: + job_label = tr['job_label'] + else: + job_label = "" #tr['transaction_id'][0:8] + click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<16}" + f"{job_label:16}{label:16}" + f"{state:<18}{time:<20}") def print_stat(response: dict, req_details): @@ -300,6 +304,9 @@ def print_meta(response:dict, req_details): "holding does not exist then it will be created with the label. " "If this option is omitted then a new holding with a random label " "will be created.") +@click.option("-b", "--job_label", default=None, type=str, + help="An optional label for the PUT job, that can be viewed when " + "using the stat command") @click.option("-i", "--holding_id", default=None, type=int, help="The numeric id of an existing holding to put the file into.") @click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, @@ -311,17 +318,11 @@ def print_meta(response:dict, req_details): "guarantee uniqueness of the holding.") @click.option("-j", "--json", default=False, type=bool, help="Output the result as JSON.") - -@click.option("-u", "--user", default=None, type=str) -@click.option("-g", "--group", default=None, type=str) -@click.option("-l", "--label", default=None, type=str) -@click.option("-i", "--holding_id", default=None, type=int) -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE) -@click.option("-j", "--json", default=False, type=bool) @click.argument("filepath", type=str) -def put(filepath, user, group, label, holding_id, tag, json): +def put(filepath, user, group, job_label, + label, holding_id, tag, json): try: - response = put_filelist([filepath], user, group, + response = put_filelist([filepath], user, group, job_label, label, holding_id, tag) if json: click.echo(response) @@ -347,12 +348,15 @@ def put(filepath, user, group, label, holding_id, tag, json): help="The username to get a file for.") @click.option("-g", "--group", default=None, type=str, help="The group to get a file for.") -@click.option("-t", "--target", default=None, type=click.Path(exists=True), +@click.option("-r", "--target", default=None, type=click.Path(exists=True), help="The target path for the retrieved file. Default is to " "retrieve the file to its original path.") @click.option("-l", "--label", default=None, type=str, help="The label of the holding to retrieve the file from. This " "can be a regular expression (regex).") +@click.option("-b", "--job_label", default=None, type=str, + help="An optional label for the GET job, that can be viewed when " + "using the stat command") @click.option("-i", "--holding_id", default=None, type=int, help="The id of the holding to retrieve the file from.") @click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, @@ -360,9 +364,10 @@ def put(filepath, user, group, label, holding_id, tag, json): @click.option("-j", "--json", default=False, type=bool, help="Output the result as JSON.") @click.argument("filepath", type=str) -def get(filepath, user, group, target, label, holding_id, tag, json): +def get(filepath, user, group, target, job_label, + label, holding_id, tag, json): try: - response = get_filelist([filepath], user, group, target, + response = get_filelist([filepath], user, group, target, job_label, label, holding_id, tag) if json: click.echo(response) @@ -390,6 +395,9 @@ def get(filepath, user, group, target, label, holding_id, tag, json): "holding does not exist then it will be created with the label. " "If this option is omitted then a new holding with a random label " "will be created.") +@click.option("-b", "--job_label", default=None, type=str, + help="An optional label for the PUT job, that can be viewed when " + "using the stat command") @click.option("-i", "--holding_id", default=None, type=int, help="The numeric id of an existing holding to put files into.") @click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, @@ -401,7 +409,8 @@ def get(filepath, user, group, target, label, holding_id, tag, json): "guarantee uniqueness of the holding.") @click.option("-j", "--json", default=False, type=bool, help="Output the result as JSON.") -def putlist(filelist, user, group, label, holding_id, tag, json): +def putlist(filelist, user, group, label, job_label, + holding_id, tag, json): # read the filelist from the file try: fh = open(filelist) @@ -411,7 +420,7 @@ def putlist(filelist, user, group, label, holding_id, tag, json): raise click.UsageError(fe) try: - response = put_filelist(files, user, group, + response = put_filelist(files, user, group, job_label, label, holding_id, tag) if json: click.echo(response) @@ -439,6 +448,9 @@ def putlist(filelist, user, group, label, holding_id, tag, json): @click.option("-l", "--label", default=None, type=str, help="The label of the holding(s) to retrieve files from. This " "can be a regular expression (regex).") +@click.option("-b", "--job_label", default=None, type=str, + help="An optional label for the GET job, that can be viewed when " + "using the stat command") @click.option("-i", "--holding_id", default=None, type=int, help="The id of the holding to retrieve files from.") @click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, @@ -446,7 +458,8 @@ def putlist(filelist, user, group, label, holding_id, tag, json): @click.option("-j", "--json", default=False, type=bool, help="Output the result as JSON.") @click.argument("filelist", type=str) -def getlist(filelist, user, group, target, label, holding_id, tag, json): +def getlist(filelist, user, group, target, job_label, + label, holding_id, tag, json): # read the filelist from the file try: fh = open(filelist) @@ -456,8 +469,8 @@ def getlist(filelist, user, group, target, label, holding_id, tag, json): raise click.UsageError(fe) try: - response = get_filelist(files, user, group, - target, label, holding_id, tag) + response = get_filelist(files, user, group, target, job_label, + label, holding_id, tag) if json: click.echo(response) else: @@ -536,7 +549,6 @@ def list(user, group, label, holding_id, tag, json): "TRANSFER_GETTING | COMPLETE | FAILED") @click.option("-j", "--json", default=False, type=bool, is_flag=True, help="Output the result as JSON.") - def stat(user, group, id, transaction_id, api_action, state, json): try: response = monitor_transactions( From 7f34d178214e22ea468a3e90bac24901f41393e3 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 24 Jan 2023 15:25:30 +0000 Subject: [PATCH 37/49] Added COMPLETE_WITH_ERRORS state --- nlds_client/clientlib/transactions.py | 9 +++++++++ nlds_client/nlds_client.py | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 5fcd75d..97f1a25 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -717,6 +717,7 @@ def get_transaction_state(transaction: dict): TRANSFER_GETTING = 6 COMPLETE = 8 FAILED = 9 + COMPLETE_WITH_ERRORS = 10 The overall state is the minimum of these """ state_mapping = { @@ -730,6 +731,7 @@ def get_transaction_state(transaction: dict): "TRANSFER_GETTING" : 6, "COMPLETE" : 8, "FAILED" : 9, + "COMPLETE_WITH_ERRORS" : 10 } state_mapping_reverse = { -1 : "INITIALISING", @@ -742,10 +744,12 @@ def get_transaction_state(transaction: dict): 6 : "TRANSFER_GETTING", 8 : "COMPLETE", 9 : "FAILED", + 10: "COMPLETE_WITH_ERRORS" } min_state = 100 min_time = datetime(1970,1,1) + error_count = 0 for sr in transaction["sub_records"]: sr_state = sr["state"] d = datetime.fromisoformat(sr["last_updated"]) @@ -753,10 +757,15 @@ def get_transaction_state(transaction: dict): min_time = d if state_mapping[sr_state] < min_state: min_state = state_mapping[sr_state] + if sr_state == "FAILED": + error_count += 1 if min_state == 100: return None, None + if min_state == state_mapping["COMPLETE"] and error_count > 0: + min_state = state_mapping["COMPLETE_WITH_ERRORS"] + return state_mapping_reverse[min_state], min_time diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 32abe5d..410fe40 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -173,7 +173,7 @@ def print_multi_stat(response: dict, req_details): stat_string += req_details click.echo(stat_string) click.echo(f"{'':<4}{'id':<6}{'action':<16}{'job label':<16}" - f"{'label':<16}{'state':<18}{'last update':<20}") + f"{'label':<16}{'state':<22}{'last update':<20}") for tr in response['data']['records']: state, time = get_transaction_state(tr) if state == None: @@ -189,7 +189,7 @@ def print_multi_stat(response: dict, req_details): job_label = "" #tr['transaction_id'][0:8] click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<16}" f"{job_label:16}{label:16}" - f"{state:<18}{time:<20}") + f"{state:<22}{time:<20}") def print_stat(response: dict, req_details): From 21b7042636bd209a0f6687b91a2f7ef1df74b048 Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Thu, 26 Jan 2023 16:38:50 +0000 Subject: [PATCH 38/49] Reorder states to reflect new duplicate catching workflow --- nlds_client/clientlib/transactions.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 97f1a25..414e39b 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -725,10 +725,11 @@ def get_transaction_state(transaction: dict): "ROUTING" : 0, "SPLITTING" : 1, "INDEXING" : 2, - "TRANSFER_PUTTING" : 3, - "CATALOG_PUTTING" : 4, - "CATALOG_GETTING" : 5, - "TRANSFER_GETTING" : 6, + "CATALOG_PUTTING" : 3, + "TRANSFER_PUTTING" : 4, + "CATALOG_ROLLBACK": 5, + "CATALOG_GETTING" : 6, + "TRANSFER_GETTING" : 7, "COMPLETE" : 8, "FAILED" : 9, "COMPLETE_WITH_ERRORS" : 10 @@ -738,10 +739,11 @@ def get_transaction_state(transaction: dict): 0 : "ROUTING", 1 : "SPLITTING", 2 : "INDEXING", - 3 : "TRANSFER_PUTTING" , - 4 : "CATALOG_PUTTING", - 5 : "CATALOG_GETTING", - 6 : "TRANSFER_GETTING", + 3 : "CATALOG_PUTTING", + 4 : "TRANSFER_PUTTING" , + 5 : "CATALOG_ROLLBACK", + 6 : "CATALOG_GETTING", + 7 : "TRANSFER_GETTING", 8 : "COMPLETE", 9 : "FAILED", 10: "COMPLETE_WITH_ERRORS" From 0d39f688a79c2fb893883848ddf3fa4906dcc6e5 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Thu, 2 Feb 2023 16:05:31 +0000 Subject: [PATCH 39/49] List holdings, find files and stat transactions on transaction_id --- nlds_client/clientlib/transactions.py | 12 ++++++++++ nlds_client/nlds_client.py | 34 ++++++++++++++++++--------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 97f1a25..81a0fd6 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -454,6 +454,7 @@ def list_holding(user: str, group: str, label: str=None, holding_id: int=None, + transaction_id: str=None, tag: dict=None): """Make a request to list the holdings in the NLDS for a user :param user: the username to get the holding(s) for @@ -499,6 +500,8 @@ def list_holding(user: str, input_params["tag"] = tag_to_string(tag) if holding_id is not None: input_params["holding_id"] = holding_id + if transaction_id is not None: + input_params["transaction_id"] = transaction_id response_dict = main_loop( url=url, @@ -522,6 +525,7 @@ def find_file(user: str, group: str, label: str=None, holding_id: int=None, + transaction_id: str=None, path: str=None, tag: dict=None): """Make a request to find files in the NLDS for a user @@ -570,6 +574,8 @@ def find_file(user: str, input_params["tag"] = tag_to_string(tag) if holding_id is not None: input_params["holding_id"] = holding_id + if transaction_id is not None: + input_params["transaction_id"] = transaction_id if path is not None: input_params["path"] = path @@ -595,6 +601,7 @@ def monitor_transactions(user: str, group: str, idd: int=None, transaction_id: str=None, + job_label: str=None, api_action: str=None, state: str=None, sub_id: str=None, @@ -614,6 +621,9 @@ def monitor_transactions(user: str, :param transaction_id: a specific transaction_id to get the status of :type transaction_id: string, optional + :param job_label: a specific job_label to get the status of + :type transaction_id: string, optional + :param api_action: applies an api-action-specific filter to the status request, only transaction_records of the given api_action will be returned. @@ -654,6 +664,8 @@ def monitor_transactions(user: str, input_params["id"] = idd if transaction_id is not None: input_params["transaction_id"] = transaction_id + if job_label is not None: + input_params["job_label"] = job_label if api_action is not None: input_params["api_action"] = api_action if sub_id is not None: diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 410fe40..ebc4dcc 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -72,7 +72,8 @@ def pretty_size(size): def format_request_details(user, group, label=None, holding_id=None, tag=None, id=None, transaction_id=None, sub_id=None, - state=None, retry_count=None, api_action=None): + state=None, retry_count=None, api_action=None, + job_label=None): config = load_config() out = "" user = get_user(config, user) @@ -91,6 +92,8 @@ def format_request_details(user, group, label=None, holding_id=None, out += f"tag:{tag}, " if transaction_id: out += f"transaction_id:{transaction_id}, " + if job_label: + out += f"job_label:{job_label}, " if sub_id: out += f"sub_id:{sub_id}, " if state: @@ -136,7 +139,8 @@ def print_list(response: dict, req_details): def print_single_stat(response: dict, req_details): """Print a single status in more detail, with a list of failed files if necessary""" - stat_string = "State of transaction: " + stat_string = "State of transaction for " + stat_string += req_details click.echo(stat_string) # still looping over the keys, just in case more than one state returned for tr in response['data']['records']: @@ -495,15 +499,17 @@ def getlist(filelist, user, group, target, job_label, "expression (regex).") @click.option("-i", "--holding_id", default=None, type=int, help="The numeric id of the holding to list.") +@click.option("-n", "--transaction_id", default=None, type=str, + help="The UUID transaction id of the transaction to list.") @click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, help="The tag(s) of the holding(s) to list.") @click.option("-j", "--json", default=False, is_flag=True, help="Output the result as JSON.") -def list(user, group, label, holding_id, tag, json): +def list(user, group, label, holding_id, transaction_id, tag, json): # try: response = list_holding( - user, group, label, holding_id, tag + user, group, label, holding_id, transaction_id, tag ) req_details = format_request_details( user, group, label=label, holding_id=holding_id, tag=tag @@ -537,22 +543,24 @@ def list(user, group, label, holding_id, tag, json): help="The group to list transactions for.") @click.option("-i", "--id", default=None, type=int, help="The numeric id of the transaction to list.") -@click.option("-T", "--transaction_id", default=None, type=str, +@click.option("-n", "--transaction_id", default=None, type=str, help="The UUID transaction id of the transaction to list.") +@click.option("-b", "--job_label", default=None, type=str, + help="The job label of the transaction(s) to list.") @click.option("-a", "--api_action", default=None, type=str, help="The api action of the transactions to list. Options: get | " "put | getlist | putlist") @click.option("-s", "--state", default=None, type=str, - help="\bThe state of the transactions to list. Options: " + help="The state of the transactions to list. Options: " "INITIALISING | ROUTING | SPLITTING | INDEXING | " "TRANSFER_PUTTING | CATALOG_PUTTING | CATALOG_GETTING | " "TRANSFER_GETTING | COMPLETE | FAILED") @click.option("-j", "--json", default=False, type=bool, is_flag=True, help="Output the result as JSON.") -def stat(user, group, id, transaction_id, api_action, state, json): +def stat(user, group, id, transaction_id, job_label, api_action, state, json): try: response = monitor_transactions( - user, group, id, transaction_id, api_action, state, + user, group, id, transaction_id, job_label, api_action, state, ) req_details = format_request_details( user, group, id=id, transaction_id=transaction_id, state=state, @@ -589,6 +597,8 @@ def stat(user, group, id, transaction_id, api_action, state, json): "can be a regular expression (regex).") @click.option("-i", "--holding_id", default=None, type=int, help="The numeric id of the holding which the files belong to.") +@click.option("-n", "--transaction_id", default=None, type=str, + help="The UUID transaction id of the transaction to list.") @click.option("-p", "--path", default=None, type=str, help="The path of the files to find. This can be a regular " "expression (regex)") @@ -596,12 +606,14 @@ def stat(user, group, id, transaction_id, api_action, state, json): help="The tag(s) of the holding(s) to find files within.") @click.option("-j", "--json", default=False, type=bool, is_flag=True, help="Output the result as JSON.") -def find(user, group, label, holding_id, path, tag, json): +def find(user, group, label, holding_id, transaction_id, path, tag, json): # try: - response = find_file(user, group, label, holding_id, path, tag) + response = find_file(user, group, label, holding_id, + transaction_id, path, tag) req_details = format_request_details( - user, group, label, holding_id, tag + user, group, label=label, holding_id=holding_id, tag=tag, + transaction_id=transaction_id ) if response['success']: if json: From 4fea0f992d7a88b4df11682b91b503737a68ca8a Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Wed, 8 Feb 2023 14:23:22 +0000 Subject: [PATCH 40/49] Add tags on upload and use meta command to add / alter tags --- nlds_client/clientlib/transactions.py | 3 ++- nlds_client/nlds_client.py | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 2181ebb..75ce64d 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -297,7 +297,8 @@ def put_filelist(filelist: List[str]=[], if label is not None: body_params["label"] = label if tag is not None: - body_params["tag"] = tag_to_string(tag) + # tags in body params should not be encoded as a string + body_params["tag"] = tag if holding_id is not None: body_params["holding_id"] = holding_id # make the request diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index ebc4dcc..00f0b19 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -126,6 +126,11 @@ def print_list(response: dict, req_details): for t in h['transactions']: trans_str += t + f"\n{'':<22}" click.echo(f"{'':<4}{'transaction id':<16}: {trans_str[:-23]}") + if 'tags' in h and len(h['tags']) > 0: + tags_str = "" + for t in h['tags']: + tags_str += f"{t} : {h['tags'][t]}\n{'':22}" + click.echo(f"{'':<4}{'tags':<16}: {tags_str[:-23]}") else: # click.echo(f"{'':<4}{'id':<6}{'label':<16}{'user':<16}{'group':<16}") click.echo(f"{'':<4}{'id':<6}{'label':<16}{'ingest time':<32}") From 9fc8c05a171614bd91e768707f732dfed155e340 Mon Sep 17 00:00:00 2001 From: Jack Leland Date: Thu, 9 Feb 2023 10:11:59 +0000 Subject: [PATCH 41/49] Differentiate transaction-level state on stat output for ease of grepping --- nlds_client/nlds_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index ebc4dcc..7d2383f 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -155,7 +155,7 @@ def print_single_stat(response: dict, req_details): if 'label' in tr: click.echo(f"{'':<4}{'label':<15}: {tr['label']}") click.echo(f"{'':<4}{'creation time':<15}: {(tr['creation_time']).replace('T',' ')}") - click.echo(f"{'':<4}{'state':<15}: {state}") + click.echo(f"{'':<4}{'job_state':<15}: {state}") click.echo(f"{'':<4}{'sub records':<15}->") for sr in tr['sub_records']: click.echo(f"{'':4}{'+':<4} {'id':<13}: {sr['id']}") From 0bae58acb4be78c1948a0a91171089299985b2dd Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Mon, 13 Feb 2023 15:40:48 +0000 Subject: [PATCH 42/49] Tag functionality --- docs/user_guide/Makefile | 20 +++++++ docs/user_guide/source/conf.py | 28 ++++++++++ nlds_client/clientlib/transactions.py | 28 +++++++--- nlds_client/nlds_client.py | 76 +++++++++++++++------------ 4 files changed, 112 insertions(+), 40 deletions(-) create mode 100644 docs/user_guide/Makefile create mode 100644 docs/user_guide/source/conf.py diff --git a/docs/user_guide/Makefile b/docs/user_guide/Makefile new file mode 100644 index 0000000..d0c3cbf --- /dev/null +++ b/docs/user_guide/Makefile @@ -0,0 +1,20 @@ +# Minimal makefile for Sphinx documentation +# + +# You can set these variables from the command line, and also +# from the environment for the first two. +SPHINXOPTS ?= +SPHINXBUILD ?= sphinx-build +SOURCEDIR = source +BUILDDIR = build + +# Put it first so that "make" without argument is like "make help". +help: + @$(SPHINXBUILD) -M help "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O) + +.PHONY: help Makefile + +# Catch-all target: route all unknown targets to Sphinx using the new +# "make mode" option. $(O) is meant as a shortcut for $(SPHINXOPTS). +%: Makefile + @$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O) diff --git a/docs/user_guide/source/conf.py b/docs/user_guide/source/conf.py new file mode 100644 index 0000000..a432b94 --- /dev/null +++ b/docs/user_guide/source/conf.py @@ -0,0 +1,28 @@ +# Configuration file for the Sphinx documentation builder. +# +# For the full list of built-in configuration values, see the documentation: +# https://www.sphinx-doc.org/en/master/usage/configuration.html + +# -- Project information ----------------------------------------------------- +# https://www.sphinx-doc.org/en/master/usage/configuration.html#project-information + +project = 'Near-Line Data Store client' +copyright = '2023, Neil Massey and Jack Leland' +author = 'Neil Massey and Jack Leland' +release = '0.1.0' + +# -- General configuration --------------------------------------------------- +# https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration + +extensions = ['sphinx_click'] + +templates_path = ['_templates'] +exclude_patterns = [] + + + +# -- Options for HTML output ------------------------------------------------- +# https://www.sphinx-doc.org/en/master/usage/configuration.html#options-for-html-output + +html_theme = 'alabaster' +html_static_path = ['_static'] diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 75ce64d..64ea89c 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -128,7 +128,7 @@ def tag_to_string(tag: dict): for key in tag: tag_str += key+":"+tag[key]+"," tag_str = tag_str.replace("'","") - return tag_str + return tag_str[:-1] def main_loop(url: str, @@ -731,6 +731,7 @@ def get_transaction_state(transaction: dict): COMPLETE = 8 FAILED = 9 COMPLETE_WITH_ERRORS = 10 + COMPLETE_WITH_WARNINGS = 11 The overall state is the minimum of these """ state_mapping = { @@ -745,7 +746,8 @@ def get_transaction_state(transaction: dict): "TRANSFER_GETTING" : 7, "COMPLETE" : 8, "FAILED" : 9, - "COMPLETE_WITH_ERRORS" : 10 + "COMPLETE_WITH_ERRORS" : 10, + "COMPLETE_WITH_WARNINGS": 11, } state_mapping_reverse = { -1 : "INITIALISING", @@ -759,7 +761,8 @@ def get_transaction_state(transaction: dict): 7 : "TRANSFER_GETTING", 8 : "COMPLETE", 9 : "FAILED", - 10: "COMPLETE_WITH_ERRORS" + 10: "COMPLETE_WITH_ERRORS", + 11: "COMPLETE_WITH_WARNINGS" } min_state = 100 @@ -781,6 +784,13 @@ def get_transaction_state(transaction: dict): if min_state == state_mapping["COMPLETE"] and error_count > 0: min_state = state_mapping["COMPLETE_WITH_ERRORS"] + # see if any warnings were given + warning_count = 0 + if "warnings" in transaction: + warning_count = len(transaction["warnings"]) + if min_state == state_mapping["COMPLETE"] and warning_count > 0: + min_state = state_mapping["COMPLETE_WITH_WARNINGS"] + return state_mapping_reverse[min_state], min_time @@ -790,7 +800,8 @@ def change_metadata(user: str, holding_id: int=None, tag: dict=None, new_label: str=None, - new_tag: dict=None): + new_tag: dict=None, + del_tag: dict=None): """Make a request to change the metadata for a NLDS holding for a user :param user: the username to change the holding(s) for :type user: string @@ -812,9 +823,12 @@ def change_metadata(user: str, :param new_label: the new label to change the label to for the holding :type new_label: str, optional - :param new_tag: the tag to add / change for the holding + :param new_tag: the tag(s) to add / change for the holding :type new_tag: dict, optional + :param del_tag: the tag(s) to delete for the holding + :type del_tag: dict, optional + :raises requests.exceptions.ConnectionError: if the server cannot be reached @@ -845,6 +859,8 @@ def change_metadata(user: str, body_params["new_label"] = new_label if new_tag is not None: body_params["new_tag"] = new_tag + if del_tag is not None: + body_params["del_tag"] = del_tag response_dict = main_loop( url=url, @@ -861,4 +877,4 @@ def change_metadata(user: str, # mark as failed in RPC call elif "details" in response_dict and "failure" in response_dict["details"]: response_dict["success"] = False - return response_dict \ No newline at end of file + return response_dict \ No newline at end of file diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 00f0b19..e211299 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -20,7 +20,9 @@ def nlds_client(): class TagParamType(click.ParamType): name = "tag" - tag_dict = {} + + def __init__(self): + self.tag_dict = {} def convert(self, value, param, ctx): try: @@ -34,9 +36,6 @@ def convert(self, value, param, ctx): ) return self.tag_dict -TAG_PARAM_TYPE = TagParamType() - - def integer_permissions_to_string(intperm): octal = oct(intperm)[2:] result = "" @@ -152,16 +151,22 @@ def print_single_stat(response: dict, req_details): state, _ = get_transaction_state(tr) if state == None: continue - click.echo(f"{'':<4}{'id':<15}: {tr['id']}") - click.echo(f"{'':<4}{'user':<15}: {tr['user']}") - click.echo(f"{'':<4}{'group':<15}: {tr['group']}") - click.echo(f"{'':<4}{'action':<15}: {tr['api_action']}") - click.echo(f"{'':<4}{'transaction id':<15}: {tr['transaction_id']}") + click.echo(f"{'':<4}{'id':<16}: {tr['id']}") + click.echo(f"{'':<4}{'user':<16}: {tr['user']}") + click.echo(f"{'':<4}{'group':<16}: {tr['group']}") + click.echo(f"{'':<4}{'action':<16}: {tr['api_action']}") + click.echo(f"{'':<4}{'transaction id':<16}: {tr['transaction_id']}") if 'label' in tr: - click.echo(f"{'':<4}{'label':<15}: {tr['label']}") - click.echo(f"{'':<4}{'creation time':<15}: {(tr['creation_time']).replace('T',' ')}") - click.echo(f"{'':<4}{'state':<15}: {state}") - click.echo(f"{'':<4}{'sub records':<15}->") + click.echo(f"{'':<4}{'label':<16}: {tr['label']}") + click.echo(f"{'':<4}{'creation time':<16}: {(tr['creation_time']).replace('T',' ')}") + click.echo(f"{'':<4}{'state':<16}: {state}") + if 'warnings' in tr: + warn_str = "" + for w in tr['warnings']: + warn_str += w + f"\n{'':<22}" + click.echo(f"{'':<4}{'warnings':<16}: {warn_str[:-23]}") + + click.echo(f"{'':<4}{'sub records':<16}->") for sr in tr['sub_records']: click.echo(f"{'':4}{'+':<4} {'id':<13}: {sr['id']}") click.echo(f"{'':<9}{'sub_id':<13}: {sr['sub_id']}") @@ -182,7 +187,7 @@ def print_multi_stat(response: dict, req_details): stat_string += req_details click.echo(stat_string) click.echo(f"{'':<4}{'id':<6}{'action':<16}{'job label':<16}" - f"{'label':<16}{'state':<22}{'last update':<20}") + f"{'label':<16}{'state':<23}{'last update':<20}") for tr in response['data']['records']: state, time = get_transaction_state(tr) if state == None: @@ -198,7 +203,7 @@ def print_multi_stat(response: dict, req_details): job_label = "" #tr['transaction_id'][0:8] click.echo(f"{'':<4}{tr['id']:<6}{tr['api_action']:<16}" f"{job_label:16}{label:16}" - f"{state:<22}{time:<20}") + f"{state:<23}{time:<20}") def print_stat(response: dict, req_details): @@ -289,16 +294,17 @@ def print_find(response:dict, req_details): def print_meta(response:dict, req_details): """Print out the response from the meta command""" - meta_string = "Changing metadata for holding for " + meta_string = "Changed metadata for holding for " meta_string += req_details click.echo(meta_string) for h in response['data']['holdings']: - click.echo("Old metadata: ") - click.echo(f"{'':<4}{'Label:':<8}{h['old_meta']['label']}") - click.echo(f"{'':<4}{'Tags:':<8}{h['old_meta']['tags']}") - click.echo("New metadata: ") - click.echo(f"{'':<4}{'Label:':<8}{h['new_meta']['label']}") - click.echo(f"{'':<4}{'Tags:':<8}{h['new_meta']['tags']}") + click.echo(f"{'':<4}{'id':<4}: {h['id']}") + click.echo(f"{'':<8}old metadata: ") + click.echo(f"{'':<12}{'label':<8}: {h['old_meta']['label']}") + click.echo(f"{'':<12}{'tags':<8}: {h['old_meta']['tags']}") + click.echo(f"{'':<8}new metadata: ") + click.echo(f"{'':<12}{'label':<8}: {h['new_meta']['label']}") + click.echo(f"{'':<12}{'tags':<8}: {h['new_meta']['tags']}") """Put files command""" @@ -318,7 +324,7 @@ def print_meta(response:dict, req_details): "using the stat command") @click.option("-i", "--holding_id", default=None, type=int, help="The numeric id of an existing holding to put the file into.") -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, +@click.option("-t", "--tag", default=None, type=TagParamType(), help="The tags of a holding to put the file into. If a holding " "with the tags already exists then the file will the added to " "that holding. If the holding does not exist then it will be " @@ -368,7 +374,7 @@ def put(filepath, user, group, job_label, "using the stat command") @click.option("-i", "--holding_id", default=None, type=int, help="The id of the holding to retrieve the file from.") -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, +@click.option("-t", "--tag", default=None, type=TagParamType(), help="The tag(s) of the holding to retrieve the file from.") @click.option("-j", "--json", default=False, type=bool, help="Output the result as JSON.") @@ -409,7 +415,7 @@ def get(filepath, user, group, target, job_label, "using the stat command") @click.option("-i", "--holding_id", default=None, type=int, help="The numeric id of an existing holding to put files into.") -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, +@click.option("-t", "--tag", default=None, type=TagParamType(), help="The tags of a holding to put files into. If a holding " "with the tags already exists then the files will the added to " "that holding. If the holding does not exist then it will be " @@ -462,7 +468,7 @@ def putlist(filelist, user, group, label, job_label, "using the stat command") @click.option("-i", "--holding_id", default=None, type=int, help="The id of the holding to retrieve files from.") -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, +@click.option("-t", "--tag", default=None, type=TagParamType(), help="The tag(s) of the holding(s) to retrieve files from.") @click.option("-j", "--json", default=False, type=bool, help="Output the result as JSON.") @@ -506,7 +512,7 @@ def getlist(filelist, user, group, target, job_label, help="The numeric id of the holding to list.") @click.option("-n", "--transaction_id", default=None, type=str, help="The UUID transaction id of the transaction to list.") -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, +@click.option("-t", "--tag", default=None, type=TagParamType(), help="The tag(s) of the holding(s) to list.") @click.option("-j", "--json", default=False, is_flag=True, help="Output the result as JSON.") @@ -607,7 +613,7 @@ def stat(user, group, id, transaction_id, job_label, api_action, state, json): @click.option("-p", "--path", default=None, type=str, help="The path of the files to find. This can be a regular " "expression (regex)") -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, +@click.option("-t", "--tag", default=None, type=TagParamType(), help="The tag(s) of the holding(s) to find files within.") @click.option("-j", "--json", default=False, type=bool, is_flag=True, help="Output the result as JSON.") @@ -651,22 +657,24 @@ def find(user, group, label, holding_id, transaction_id, path, tag, json): "be a regular expression (regex)") @click.option("-i", "--holding_id", default=None, type=int, help="The numeric id of the holding to change metadata for.") -@click.option("-t", "--tag", default=None, type=TAG_PARAM_TYPE, +@click.option("-t", "--tag", default=None, type=TagParamType(), help="The tag(s) of the holding(s) to change metadata for.") @click.option("-L", "--new_label", default=None, type=str, help="The new label for the holding.") -@click.option("-T", "--new_tag", default=None, type=TAG_PARAM_TYPE, +@click.option("-T", "--new_tag", default=None, type=TagParamType(), help="The new tag(s) for the holding.") +@click.option("-D", "--del_tag", default=None, type=TagParamType(), + help="Delete a tag from the holding.") @click.option("-j", "--json", default=False, type=bool, is_flag=True, help="Output the result as JSON.") -def meta(user, group, label, holding_id, tag, new_label, new_tag, json): - # +def meta(user, group, label, holding_id, tag, new_label, new_tag, del_tag, json): + # try: - response = change_metadata(user, group, label, holding_id, tag, - new_label, new_tag) req_details = format_request_details( user, group, label, holding_id, tag ) + response = change_metadata(user, group, label, holding_id, tag, + new_label, new_tag, del_tag) if response['success'] > 0: if json: click.echo(response) From 905f3bb85110208fd9fe816be407b55e0d49e7a1 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 21 Feb 2023 16:56:02 +0000 Subject: [PATCH 43/49] added __init__.py in nlds_client --- nlds_client/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 nlds_client/__init__.py diff --git a/nlds_client/__init__.py b/nlds_client/__init__.py new file mode 100644 index 0000000..e69de29 From 3ba02def94216c8c7d9d0f46c9df08a1f5ee6175 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 21 Feb 2023 16:59:17 +0000 Subject: [PATCH 44/49] Added nlds_client/clientlib/__init__.py --- nlds_client/clientlib/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 nlds_client/clientlib/__init__.py diff --git a/nlds_client/clientlib/__init__.py b/nlds_client/clientlib/__init__.py new file mode 100644 index 0000000..e69de29 From cd79358ed1bdd4195abd82665f8c785c854a0c30 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 21 Feb 2023 17:01:45 +0000 Subject: [PATCH 45/49] changed name of package in setup.py --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 62ae4ad..e3a3414 100644 --- a/setup.py +++ b/setup.py @@ -8,7 +8,7 @@ os.chdir(os.path.normpath(os.path.join(os.path.abspath(__file__), os.pardir))) setup( - name='nlds-client', + name='nlds_client', version='0.0.2', packages=['nlds_client'], install_requires=[ From b5603dead2ea99fdad9b032faf590c2b4571f5f3 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Tue, 21 Feb 2023 17:07:47 +0000 Subject: [PATCH 46/49] added find_packages (finally fixed install problem) --- setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index e3a3414..f7e8a26 100644 --- a/setup.py +++ b/setup.py @@ -1,5 +1,5 @@ import os -from setuptools import setup +from setuptools import setup, find_packages with open(os.path.join(os.path.dirname(__file__), 'README.md')) as readme: README = readme.read() @@ -10,7 +10,7 @@ setup( name='nlds_client', version='0.0.2', - packages=['nlds_client'], + packages=find_packages(), install_requires=[ 'requests', 'requests_oauthlib', From 7eb627cd19f21d72e36a9ff1cb74d7b0c71c8672 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Wed, 1 Mar 2023 14:42:34 +0000 Subject: [PATCH 47/49] Added nicer return messages for PUT and GET --- nlds_client/nlds_client.py | 43 +++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index e211299..1aaeb40 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -104,6 +104,13 @@ def format_request_details(user, group, label=None, holding_id=None, return out[:-2] +def _tags_to_str(tags): + tags_str = "" + for t in h['tags']: + tags_str += f"{t} : {h['tags'][t]}\n{'':22}" + return tags_str + + def print_list(response: dict, req_details): """Print out the response from the list command""" n_holdings = len(response['data']['holdings']) @@ -126,9 +133,7 @@ def print_list(response: dict, req_details): trans_str += t + f"\n{'':<22}" click.echo(f"{'':<4}{'transaction id':<16}: {trans_str[:-23]}") if 'tags' in h and len(h['tags']) > 0: - tags_str = "" - for t in h['tags']: - tags_str += f"{t} : {h['tags'][t]}\n{'':22}" + tags_str = _tags_to_str(h['tags']) click.echo(f"{'':<4}{'tags':<16}: {tags_str[:-23]}") else: # click.echo(f"{'':<4}{'id':<6}{'label':<16}{'user':<16}{'group':<16}") @@ -292,7 +297,7 @@ def print_find(response:dict, req_details): print_multi_file(response) -def print_meta(response:dict, req_details): +def print_meta(response:dict, req_details:str): """Print out the response from the meta command""" meta_string = "Changed metadata for holding for " meta_string += req_details @@ -307,6 +312,28 @@ def print_meta(response:dict, req_details): click.echo(f"{'':<12}{'tags':<8}: {h['new_meta']['tags']}") +def print_response(tr:dict): + if 'msg' in tr and len(tr['msg']) > 0: + click.echo(tr['msg']) + if 'holding_id' in tr and tr['holding_id'] > 0: + click.echo(f"{'':<4}{'id':<16}: {tr['holding_id']}") + if 'user' in tr and len(tr['user']) > 0: + click.echo(f"{'':<4}{'user':<16}: {tr['user']}") + if 'group' in tr and len(tr['group']) > 0: + click.echo(f"{'':<4}{'group':<16}: {tr['group']}") + if 'api_action' in tr and len(tr['api_action']) > 0: + click.echo(f"{'':<4}{'action':<16}: {tr['api_action']}") + if 'job_label' in tr and len(tr['job_label']) > 0: + click.echo(f"{'':<4}{'job label':<16}: {tr['job_label']}") + if 'transaction_id' in tr and len(tr['transaction_id']) > 0: + click.echo(f"{'':<4}{'transaction id':<16}: {tr['transaction_id']}") + if 'label' in tr and len(tr['label']) > 0: + click.echo(f"{'':<4}{'label':<16}: {tr['label']}") + if 'tag' in tr and len(tr['tag']) > 0: + tag_str = _tags_to_str(tr['tag']) + click.echo(f"{'':<4}{'tags':<16}: {tag_str}") + + """Put files command""" @nlds_client.command("put", help="Put a single file.") @click.option("-u", "--user", default=None, type=str, @@ -342,7 +369,7 @@ def put(filepath, user, group, job_label, if json: click.echo(response) else: - click.echo(response['msg'].strip('\n')) + print_response(response) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -387,7 +414,7 @@ def get(filepath, user, group, target, job_label, if json: click.echo(response) else: - click.echo(response['msg'].strip('\n')) + print_response(response) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: @@ -440,7 +467,7 @@ def putlist(filelist, user, group, label, job_label, if json: click.echo(response) else: - click.echo(response['msg'].strip('\n')) + print_response(response) except ConnectionError as ce: raise click.UsageError(ce) @@ -489,7 +516,7 @@ def getlist(filelist, user, group, target, job_label, if json: click.echo(response) else: - click.echo(response['msg'].strip('\n')) + print_response(response) except ConnectionError as ce: raise click.UsageError(ce) except AuthenticationError as ae: From 47fc525a5464df6e8ef0b95bc3f9ac3cc06209cd Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Wed, 1 Mar 2023 14:45:27 +0000 Subject: [PATCH 48/49] Corrected error in _tags_to_str --- nlds_client/nlds_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nlds_client/nlds_client.py b/nlds_client/nlds_client.py index 1aaeb40..9965c15 100755 --- a/nlds_client/nlds_client.py +++ b/nlds_client/nlds_client.py @@ -106,8 +106,8 @@ def format_request_details(user, group, label=None, holding_id=None, def _tags_to_str(tags): tags_str = "" - for t in h['tags']: - tags_str += f"{t} : {h['tags'][t]}\n{'':22}" + for t in tags: + tags_str += f"{t} : {tags[t]}\n{'':22}" return tags_str From 0784597e499c504fe8d52c3fbd0ec86125d986d7 Mon Sep 17 00:00:00 2001 From: Neil Massey Date: Wed, 8 Mar 2023 08:15:17 +0000 Subject: [PATCH 49/49] Added a comment about the return from monitor_transactions --- nlds_client/clientlib/transactions.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nlds_client/clientlib/transactions.py b/nlds_client/clientlib/transactions.py index 64ea89c..5417c9d 100644 --- a/nlds_client/clientlib/transactions.py +++ b/nlds_client/clientlib/transactions.py @@ -699,6 +699,8 @@ def get_transaction_state(transaction: dict): """Get the overall state of a transaction in a more convienent form by querying the sub-transactions and determining if the overall transaction is complete. + transaction: a dictionary for a single transaction. Note that + monitor_transactions returns a dictionary of transactions Transaction dictionary looks like this: { 'id': 2,