From 5dd5f3d40c958826ffb61125823c0864e65252eb Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Tue, 19 Nov 2024 11:43:28 +0100 Subject: [PATCH 01/20] Add Authentication Provider Enum --- mars-cli/mars_lib/authentication.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/mars-cli/mars_lib/authentication.py b/mars-cli/mars_lib/authentication.py index eaf0bec..54a26eb 100644 --- a/mars-cli/mars_lib/authentication.py +++ b/mars-cli/mars_lib/authentication.py @@ -1,6 +1,21 @@ from typing import Optional import requests import json +from enum import Enum + + +class AuthProvider(Enum): + """ + Holds constants, tied to the repository authentication providers. + """ + + WEBIN = "webin" + METABOLIGHTS_METADATA = "metabolights_metadata" + METABOLIGHTS_DATA = "metabolights_data" + + @classmethod + def available_providers(cls): + return {item.value for item in cls} def get_webin_auth_token( From c373b58a7672f2f02df04c20beb0b13fc9d1c261 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Tue, 19 Nov 2024 11:45:59 +0100 Subject: [PATCH 02/20] Remove header overloading --- mars-cli/mars_lib/authentication.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/mars-cli/mars_lib/authentication.py b/mars-cli/mars_lib/authentication.py index 54a26eb..936e6b7 100644 --- a/mars-cli/mars_lib/authentication.py +++ b/mars-cli/mars_lib/authentication.py @@ -60,7 +60,10 @@ def get_webin_auth_token( def get_metabolights_auth_token( credentials_dict: dict[str, str], - headers: dict[str, str] = {"Content-Type": "application/x-www-form-urlencoded"}, + headers: dict[str, str] = { + "Content-Type": "application/x-www-form-urlencoded", + "Accept": "application/json", + }, auth_url: str = "https://www-test.ebi.ac.uk/metabolights/mars/ws3/auth/token", ) -> Optional[str]: """ @@ -74,10 +77,6 @@ def get_metabolights_auth_token( Returns: str: The obtained token. """ - headers = { - "Content-Type": "application/x-www-form-urlencoded", - "Accept": "application/json", - } form_data = f'grant_type=password&username={credentials_dict["username"]}&password={credentials_dict["password"]}' try: response = requests.post( From c549803f03bfe3a9afd6f626a78d1b6995f2c365 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 21 Nov 2024 14:01:33 +0100 Subject: [PATCH 03/20] Update credentials file to hold multiple credentials --- mars-cli/mars_lib/authentication.py | 43 ++++++++++++++++- mars-cli/mars_lib/submit.py | 14 ++++-- mars-cli/tests/test_authentication.py | 49 ++++++++++++++++---- mars-cli/tests/test_credentials_example.json | 14 +++++- 4 files changed, 102 insertions(+), 18 deletions(-) diff --git a/mars-cli/mars_lib/authentication.py b/mars-cli/mars_lib/authentication.py index 936e6b7..d12b296 100644 --- a/mars-cli/mars_lib/authentication.py +++ b/mars-cli/mars_lib/authentication.py @@ -1,4 +1,5 @@ -from typing import Optional +import io +from typing import Optional, Union import requests import json from enum import Enum @@ -18,6 +19,46 @@ def available_providers(cls): return {item.value for item in cls} +def load_credentials( + credentials_file: Union[io.TextIOWrapper, str] +) -> dict[str, dict[str, str]]: + """ + Validate the credentials. + + Args: + credentials_file (_): The credentials in file formate. + + Raises: + ValueError: If the credentials are not valid. + + Returns: + dict: The credentials. + """ + if type(credentials_file) == str: + with open(credentials_file, "r") as file: + credentials = json.load(file) + elif type(credentials_file) == io.TextIOWrapper: + with open(credentials_file.name, "r") as file: + credentials = json.load(file) + else: + raise TypeError("Credentials file must be of type str or io.TextIOWrapper.") + + if not all( + repo in AuthProvider.available_providers() for repo in credentials.keys() + ): + raise ValueError("Credentials dictionary must have keys .") + + if not all( + key in ["username", "password"] + for repo, creds in credentials.items() + for key in creds.keys() + ): + raise ValueError( + "Credentials dictionary must contain 'username' and 'password' keys." + ) + return credentials + + def get_webin_auth_token( credentials_dict: dict[str, str], header: dict[str, str] = {"Content-Type": "application/json"}, diff --git a/mars-cli/mars_lib/submit.py b/mars-cli/mars_lib/submit.py index cf86b66..215a70b 100644 --- a/mars-cli/mars_lib/submit.py +++ b/mars-cli/mars_lib/submit.py @@ -6,7 +6,11 @@ import requests import json from typing import Any -from mars_lib.authentication import get_metabolights_auth_token, get_webin_auth_token +from mars_lib.authentication import ( + get_metabolights_auth_token, + get_webin_auth_token, + load_credentials, +) from mars_lib.biosamples_external_references import ( get_header, biosamples_endpoints, @@ -69,7 +73,7 @@ def submission( if credentials_file == "": raise ValueError("No credentials found") - user_credentials = json.load(credentials_file) + user_credentials = load_credentials(credentials_file) isa_json = load_isa_json(isa_json_file, investigation_is_root) @@ -252,9 +256,9 @@ def upload_to_metabolights( "accept": "application/json", "Authorization": f"Bearer {token}", } - isa_json_str = isa_json.investigation.model_dump_json( - by_alias=True, exclude_none=True - ) + isa_json_str = reduce_isa_json_for_target_repo( + isa_json, TargetRepository.METABOLIGHTS + ).investigation.model_dump_json(by_alias=True, exclude_none=True) json_file = io.StringIO(isa_json_str) files = {"isa_json_file": ("isa_json.json", json_file)} diff --git a/mars-cli/tests/test_authentication.py b/mars-cli/tests/test_authentication.py index f7cc6c5..ac7481f 100644 --- a/mars-cli/tests/test_authentication.py +++ b/mars-cli/tests/test_authentication.py @@ -1,24 +1,53 @@ import pytest -import json import os -from mars_lib.authentication import get_webin_auth_token +from mars_lib.authentication import ( + get_webin_auth_token, + load_credentials, + get_metabolights_auth_token, +) +from requests.exceptions import HTTPError - -def test_get_webin_auth_token(): - fake_credentials_dict = { +fake_credentials_dict = { + "webin": { + "username": "my_fake_account", + "password": "my_super_secret_password", + }, + "metabolights_metadata": { + "username": "my_fake_account", + "password": "my_super_secret_password", + }, + "metabolights_data": { "username": "my_fake_account", "password": "my_super_secret_password", - } + }, +} + + +def test_get_webin_auth_token(): with pytest.raises( ValueError, match="ERROR when generating token. See response's content below:\nBad credentials", ): - get_webin_auth_token(fake_credentials_dict) + get_webin_auth_token(fake_credentials_dict["webin"]) file = "./tests/test_credentials.json" if os.path.exists(file) and os.path.isfile(file): - with open(file, "r") as f: - test_credentials = json.load(f) + test_credentials = load_credentials(file) - token = get_webin_auth_token(test_credentials) + token = get_webin_auth_token(test_credentials["webin"]) assert token + + +@pytest.mark.skipif( + not os.path.exists("./tests/test_credentials.json"), + reason="Credentials file not found", +) +def test_get_metabolights_auth_token(): + credentials = load_credentials("./tests/test_credentials.json") + token = get_metabolights_auth_token(credentials["metabolights_metadata"]) + assert token + + # TODO: Currently an 'Internal Server Error' is returned when using the wrong credentials. + # This should be updated to return a more informative error message. + with pytest.raises(HTTPError): + get_metabolights_auth_token(fake_credentials_dict["metabolights_metadata"]) diff --git a/mars-cli/tests/test_credentials_example.json b/mars-cli/tests/test_credentials_example.json index f3600fb..d7eafb4 100644 --- a/mars-cli/tests/test_credentials_example.json +++ b/mars-cli/tests/test_credentials_example.json @@ -1,4 +1,14 @@ { - "username": "put-your-username-here", - "password": "put-your-password-here" + "webin": { + "username": "put-your-username-here", + "password": "put-your-password-here" + }, + "metabolights_metadata": { + "username": "put-your-username-here", + "password": "put-your-password-here" + }, + "metabolights_data": { + "username": "put-your-username-here", + "password": "put-your-password-here" + } } \ No newline at end of file From e9666a6bc5a13ac8ee81d32402c185d8aafe925f Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 21 Nov 2024 16:00:49 +0100 Subject: [PATCH 04/20] Add validation for credential manager --- mars-cli/mars_lib/authentication.py | 4 ++++ mars-cli/mars_lib/credential.py | 9 +++++++-- mars-cli/tests/test_credential_manager.py | 9 ++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/mars-cli/mars_lib/authentication.py b/mars-cli/mars_lib/authentication.py index d12b296..7b76337 100644 --- a/mars-cli/mars_lib/authentication.py +++ b/mars-cli/mars_lib/authentication.py @@ -18,6 +18,10 @@ class AuthProvider(Enum): def available_providers(cls): return {item.value for item in cls} + @classmethod + def is_valid_provider(cls, provider: str): + return provider in cls.available_providers() + def load_credentials( credentials_file: Union[io.TextIOWrapper, str] diff --git a/mars-cli/mars_lib/credential.py b/mars-cli/mars_lib/credential.py index d3135c0..cb9c3bf 100644 --- a/mars-cli/mars_lib/credential.py +++ b/mars-cli/mars_lib/credential.py @@ -3,6 +3,8 @@ import getpass import keyring.util.platform_ as keyring_platform +from mars_lib.authentication import AuthProvider + """ Credential Manager Module ========================= @@ -52,8 +54,11 @@ class CredentialManager: - def __init__(self, service_name: str) -> None: - self.service_name = service_name + def __init__(self, auth_provider: str) -> None: + if not AuthProvider.is_valid_provider(auth_provider): + raise ValueError(f"Invalid authentication provider: {auth_provider}") + + self.service_name = auth_provider def get_credential_env(self, username: str) -> str: """ diff --git a/mars-cli/tests/test_credential_manager.py b/mars-cli/tests/test_credential_manager.py index fd08371..55fa494 100644 --- a/mars-cli/tests/test_credential_manager.py +++ b/mars-cli/tests/test_credential_manager.py @@ -1,10 +1,17 @@ +import pytest + from mars_lib.credential import CredentialManager def test_create_credentials_manager(): - cm = CredentialManager("mars-cli") + cm = CredentialManager("webin") assert cm is not None + with pytest.raises( + ValueError, match="Invalid authentication provider: invalid_provider" + ): + CredentialManager("invalid_provider") + def test_set_password_keyring(): cm = CredentialManager("mars-cli") From 1fb120418c4e7b4268d5a32a8884275edc88b7bb Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 21 Nov 2024 16:00:58 +0100 Subject: [PATCH 05/20] Add options --- mars-cli/mars_cli.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/mars-cli/mars_cli.py b/mars-cli/mars_cli.py index e6596af..d3b8be7 100644 --- a/mars-cli/mars_cli.py +++ b/mars-cli/mars_cli.py @@ -197,10 +197,17 @@ def cli(ctx, development): @cli.command() @click.option( - "--credential-service-name", type=click.STRING, help="service name from the keyring" + "--webin-username", type=click.STRING, help="Username for webin authentication" ) @click.option( - "--username-credentials", type=click.STRING, help="Username from the keyring" + "--metabolights-username", + type=click.STRING, + help="Username for MetaboLights metadata submission", +) +@click.option( + "--metabolights-ftp-username", + type=click.STRING, + help="Username for MetaboLights data submission", ) @click.option( "--credentials-file", @@ -247,8 +254,9 @@ def cli(ctx, development): @click.pass_context def submit( ctx, - credential_service_name, - username_credentials, + webin_username, + metabolights_username, + metabolights_ftp_username, credentials_file, isa_json_file, submit_to_biosamples, From a9e29e92e6b44c2185094eddde6084b5ebc0ab1a Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 21 Nov 2024 16:26:16 +0100 Subject: [PATCH 06/20] Implement in submission function --- mars-cli/mars_cli.py | 12 +++++++++--- mars-cli/mars_lib/submit.py | 21 +++++++++++++++------ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/mars-cli/mars_cli.py b/mars-cli/mars_cli.py index d3b8be7..ffdb077 100644 --- a/mars-cli/mars_cli.py +++ b/mars-cli/mars_cli.py @@ -197,17 +197,22 @@ def cli(ctx, development): @cli.command() @click.option( - "--webin-username", type=click.STRING, help="Username for webin authentication" + "--webin-username", + type=click.STRING, + help="Username for webin authentication", + envvar="WEBIN_USERNAME", ) @click.option( "--metabolights-username", type=click.STRING, help="Username for MetaboLights metadata submission", + envvar="METABOLIGHTS_USERNAME", ) @click.option( "--metabolights-ftp-username", type=click.STRING, help="Username for MetaboLights data submission", + envvar="METABOLIGHTS_FTP_USERNAME", ) @click.option( "--credentials-file", @@ -288,8 +293,9 @@ def submit( data_file_paths = [f.name for f in data_files] if file_transfer else [] submission( - credential_service_name, - username_credentials, + webin_username, + metabolights_username, + metabolights_ftp_username, credentials_file, isa_json_file.name, target_repositories, diff --git a/mars-cli/mars_lib/submit.py b/mars-cli/mars_lib/submit.py index 215a70b..6cf3dcb 100644 --- a/mars-cli/mars_lib/submit.py +++ b/mars-cli/mars_lib/submit.py @@ -10,6 +10,7 @@ get_metabolights_auth_token, get_webin_auth_token, load_credentials, + AuthProvider, ) from mars_lib.biosamples_external_references import ( get_header, @@ -48,8 +49,9 @@ def save_step_to_file(time_stamp: float, filename: str, isa_json: IsaJson): def submission( - credential_service_name: str, - username_credentials: str, + webin_username: str, + metabolights_username: str, + metabolights_ftp_username: str, credentials_file: TextIOWrapper, isa_json_file: str, target_repositories: list[str], @@ -63,11 +65,18 @@ def submission( # Get password from the credential manager # Else: # read credentials from file - if not (credential_service_name is None or username_credentials is None): - cm = CredentialManager(credential_service_name) + if all([webin_username, metabolights_username, metabolights_ftp_username]): user_credentials = { - "username": username_credentials, - "password": cm.get_password_keyring(username_credentials), + cred_pair[0]: { + "username": cred_pair[1], + "password": CredentialManager(cred_pair[0]).get_password_keyring( + cred_pair[1] + ), + } + for cred_pair in zip( + AuthProvider.available_providers(), + [webin_username, metabolights_username, metabolights_ftp_username], + ) } else: if credentials_file == "": From 08b527be9efc3f9f827a5cd63bad6de232d178de Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 21 Nov 2024 16:42:02 +0100 Subject: [PATCH 07/20] Modify set-password command --- mars-cli/mars_cli.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mars-cli/mars_cli.py b/mars-cli/mars_cli.py index ffdb077..9b3d359 100644 --- a/mars-cli/mars_cli.py +++ b/mars-cli/mars_cli.py @@ -375,12 +375,12 @@ def validate_isa_json(isa_json_file, investigation_is_root, validation_schema): @cli.command() @click.option( - "--service-name", - type=click.STRING, + "--auth-provider", + type=click.Choice(['webin', 'metabolights_metadata', 'metabolights_data'], case_sensitive=False), is_flag=False, flag_value="value", - default=f"mars-cli_{datetime.now().strftime('%Y-%m-%dT%H:%M:%S')}", - help='You are advised to include service name to match the credentials to. If empty, it defaults to "mars-cli_{DATESTAMP}"', + required=True, + help='', ) @click.argument( "username", @@ -394,9 +394,9 @@ def validate_isa_json(isa_json_file, investigation_is_root, validation_schema): confirmation_prompt=True, help="The password to store. Note: You are required to confirm the password.", ) -def set_password(service_name, username, password): +def set_password(auth_provider, username, password): """Store a password in the keyring.""" - CredentialManager(service_name).set_password_keyring(username, password) + CredentialManager(auth_provider).set_password_keyring(username, password) if __name__ == "__main__": From a638dd2ad5d00afb7d0c62482306154ca76e171e Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 21 Nov 2024 16:44:32 +0100 Subject: [PATCH 08/20] Fix test --- mars-cli/tests/test_credential_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mars-cli/tests/test_credential_manager.py b/mars-cli/tests/test_credential_manager.py index 55fa494..64b5698 100644 --- a/mars-cli/tests/test_credential_manager.py +++ b/mars-cli/tests/test_credential_manager.py @@ -14,6 +14,6 @@ def test_create_credentials_manager(): def test_set_password_keyring(): - cm = CredentialManager("mars-cli") + cm = CredentialManager("webin") cm.set_password_keyring("username", "password") assert cm.get_password_keyring("username") == "password" From 6af4f3dbd60adcdb424038fa6edf74cbcf0b5ad3 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 21 Nov 2024 16:49:06 +0100 Subject: [PATCH 09/20] formatting, linting and type checking --- mars-cli/mars_cli.py | 6 ++++-- mars-cli/mars_lib/authentication.py | 4 ++-- mars-cli/mars_lib/submit.py | 10 ++++++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/mars-cli/mars_cli.py b/mars-cli/mars_cli.py index 9b3d359..dc23745 100644 --- a/mars-cli/mars_cli.py +++ b/mars-cli/mars_cli.py @@ -376,11 +376,13 @@ def validate_isa_json(isa_json_file, investigation_is_root, validation_schema): @cli.command() @click.option( "--auth-provider", - type=click.Choice(['webin', 'metabolights_metadata', 'metabolights_data'], case_sensitive=False), + type=click.Choice( + ["webin", "metabolights_metadata", "metabolights_data"], case_sensitive=False + ), is_flag=False, flag_value="value", required=True, - help='', + help="", ) @click.argument( "username", diff --git a/mars-cli/mars_lib/authentication.py b/mars-cli/mars_lib/authentication.py index 7b76337..247deb4 100644 --- a/mars-cli/mars_lib/authentication.py +++ b/mars-cli/mars_lib/authentication.py @@ -38,10 +38,10 @@ def load_credentials( Returns: dict: The credentials. """ - if type(credentials_file) == str: + if isinstance(credentials_file, str): with open(credentials_file, "r") as file: credentials = json.load(file) - elif type(credentials_file) == io.TextIOWrapper: + elif isinstance(credentials_file, io.TextIOWrapper): with open(credentials_file.name, "r") as file: credentials = json.load(file) else: diff --git a/mars-cli/mars_lib/submit.py b/mars-cli/mars_lib/submit.py index 6cf3dcb..794e551 100644 --- a/mars-cli/mars_lib/submit.py +++ b/mars-cli/mars_lib/submit.py @@ -114,7 +114,7 @@ def submission( # Submit to Biosamples biosamples_result = submit_to_biosamples( isa_json=isa_json, - biosamples_credentials=user_credentials, + biosamples_credentials=user_credentials[TargetRepository.BIOSAMPLES.value], biosamples_url=urls["BIOSAMPLES"]["SUBMISSION"], webin_token_url=urls["WEBIN"]["TOKEN"], ) @@ -137,7 +137,7 @@ def submission( file_paths=[ Path(df) for df in data_file_map[TargetRepository.ENA.value] ], - user_credentials=user_credentials, + user_credentials=user_credentials[TargetRepository.ENA.value], submission_url=urls["ENA"]["DATA-SUBMISSION"], file_transfer=file_transfer, ) @@ -148,7 +148,7 @@ def submission( # Step 2 : submit isa-json to ena ena_result = submit_to_ena( isa_json=isa_json, - user_credentials=user_credentials, + user_credentials=user_credentials[TargetRepository.ENA.value], submission_url=urls["ENA"]["SUBMISSION"], ) print_and_log( @@ -172,7 +172,9 @@ def submission( file_paths=data_file_map[TargetRepository.METABOLIGHTS.value], file_transfer=file_transfer, isa_json=isa_json, - metabolights_credentials=user_credentials, + metabolights_credentials=user_credentials[ + TargetRepository.METABOLIGHTS.value + ], metabolights_url=urls["METABOLIGHTS"]["SUBMISSION"], metabolights_token_url=urls["METABOLIGHTS"]["TOKEN"], ) From f8abb1956f5d6741ffcacd37170fe2c2d12eb1d5 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 21 Nov 2024 17:13:17 +0100 Subject: [PATCH 10/20] Remove default fields --- mars-cli/mars_lib/models/isa_json.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mars-cli/mars_lib/models/isa_json.py b/mars-cli/mars_lib/models/isa_json.py index 2eed601..456f7b3 100644 --- a/mars-cli/mars_lib/models/isa_json.py +++ b/mars-cli/mars_lib/models/isa_json.py @@ -255,7 +255,7 @@ class MaterialAttribute(IsaBase): class Study(CommentedIsaBase): - id: str = Field(alias="@id", default=None) + id: str = Field(alias="@id") assays: List[Assay] = [] characteristicCategories: List[MaterialAttribute] = [] description: Optional[str] = None @@ -284,7 +284,7 @@ def validate_filename(cls, v: str) -> Union[str, None]: class Investigation(CommentedIsaBase): - id: str = Field(alias="@id", default=None) + id: str = Field(alias="@id") description: Optional[str] = None filename: Optional[str] = None identifier: Optional[str] = None From bc31ad3a4fc401e543321e166b4ccf8c3ef07912 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 21 Nov 2024 17:17:52 +0100 Subject: [PATCH 11/20] Revert change --- mars-cli/mars_lib/models/isa_json.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mars-cli/mars_lib/models/isa_json.py b/mars-cli/mars_lib/models/isa_json.py index 456f7b3..6efa5e9 100644 --- a/mars-cli/mars_lib/models/isa_json.py +++ b/mars-cli/mars_lib/models/isa_json.py @@ -255,7 +255,7 @@ class MaterialAttribute(IsaBase): class Study(CommentedIsaBase): - id: str = Field(alias="@id") + id: Optional[str] = Field(alias="@id", default=None) assays: List[Assay] = [] characteristicCategories: List[MaterialAttribute] = [] description: Optional[str] = None @@ -284,7 +284,7 @@ def validate_filename(cls, v: str) -> Union[str, None]: class Investigation(CommentedIsaBase): - id: str = Field(alias="@id") + id: Optional[str] = Field(alias="@id", default=None) description: Optional[str] = None filename: Optional[str] = None identifier: Optional[str] = None From ec0fd4f56f05109616bf5201d0b3e23652781b7c Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 2 Jan 2025 10:12:14 +0100 Subject: [PATCH 12/20] Add test that tests the credentials file --- mars-cli/mars_lib/authentication.py | 3 ++- .../tests/fixtures/bad_credentials_file.json | 18 ++++++++++++++++++ .../tests/fixtures/max_credentials_file.json | 14 ++++++++++++++ .../tests/fixtures/min_credentials_file.json | 6 ++++++ mars-cli/tests/test_authentication.py | 12 ++++++++++++ 5 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 mars-cli/tests/fixtures/bad_credentials_file.json create mode 100644 mars-cli/tests/fixtures/max_credentials_file.json create mode 100644 mars-cli/tests/fixtures/min_credentials_file.json diff --git a/mars-cli/mars_lib/authentication.py b/mars-cli/mars_lib/authentication.py index 247deb4..b41582f 100644 --- a/mars-cli/mars_lib/authentication.py +++ b/mars-cli/mars_lib/authentication.py @@ -50,7 +50,8 @@ def load_credentials( if not all( repo in AuthProvider.available_providers() for repo in credentials.keys() ): - raise ValueError("Credentials dictionary must have keys .") + raise ValueError( + f"Credentials dictionary must have valid keys. Valid keys are:\n{AuthProvider.available_providers()}") if not all( key in ["username", "password"] diff --git a/mars-cli/tests/fixtures/bad_credentials_file.json b/mars-cli/tests/fixtures/bad_credentials_file.json new file mode 100644 index 0000000..6b7b8f4 --- /dev/null +++ b/mars-cli/tests/fixtures/bad_credentials_file.json @@ -0,0 +1,18 @@ +{ + "webin": { + "username": "put-your-username-here", + "password": "put-your-password-here" + }, + "metabolights_metadata": { + "username": "put-your-username-here", + "password": "put-your-password-here" + }, + "metabolights_data": { + "username": "put-your-username-here", + "password": "put-your-password-here" + }, + "blahblahblah_repo": { + "username": "put-your-username", + "password": "put-your-password" + } +} \ No newline at end of file diff --git a/mars-cli/tests/fixtures/max_credentials_file.json b/mars-cli/tests/fixtures/max_credentials_file.json new file mode 100644 index 0000000..d7eafb4 --- /dev/null +++ b/mars-cli/tests/fixtures/max_credentials_file.json @@ -0,0 +1,14 @@ +{ + "webin": { + "username": "put-your-username-here", + "password": "put-your-password-here" + }, + "metabolights_metadata": { + "username": "put-your-username-here", + "password": "put-your-password-here" + }, + "metabolights_data": { + "username": "put-your-username-here", + "password": "put-your-password-here" + } +} \ No newline at end of file diff --git a/mars-cli/tests/fixtures/min_credentials_file.json b/mars-cli/tests/fixtures/min_credentials_file.json new file mode 100644 index 0000000..2735046 --- /dev/null +++ b/mars-cli/tests/fixtures/min_credentials_file.json @@ -0,0 +1,6 @@ +{ + "webin": { + "username": "put-your-username-here", + "password": "put-your-password-here" + } +} \ No newline at end of file diff --git a/mars-cli/tests/test_authentication.py b/mars-cli/tests/test_authentication.py index ac7481f..6957915 100644 --- a/mars-cli/tests/test_authentication.py +++ b/mars-cli/tests/test_authentication.py @@ -51,3 +51,15 @@ def test_get_metabolights_auth_token(): # This should be updated to return a more informative error message. with pytest.raises(HTTPError): get_metabolights_auth_token(fake_credentials_dict["metabolights_metadata"]) + + +def test_valid_credentials_file(): + # Test with a full valid credentials file (all providers) + _max_credentials = load_credentials("tests/fixtures/max_credentials_file.json") + + # Test with a partial valid credentials file + _min_credentials = load_credentials("tests/fixtures/min_credentials_file.json") + + # Test with a credentials file that has an invalid provider + with pytest.raises(ValueError, match="Credentials dictionary must have valid keys."): + load_credentials("tests/fixtures/bad_credentials_file.json") From 1eeb48f74fc37598723b28e0d553e6e483fbd1e5 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 2 Jan 2025 10:47:16 +0100 Subject: [PATCH 13/20] Correct user credentials --- mars-cli/mars_lib/authentication.py | 3 ++- mars-cli/mars_lib/submit.py | 8 ++++---- mars-cli/tests/test_authentication.py | 4 +++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/mars-cli/mars_lib/authentication.py b/mars-cli/mars_lib/authentication.py index b41582f..3a66845 100644 --- a/mars-cli/mars_lib/authentication.py +++ b/mars-cli/mars_lib/authentication.py @@ -51,7 +51,8 @@ def load_credentials( repo in AuthProvider.available_providers() for repo in credentials.keys() ): raise ValueError( - f"Credentials dictionary must have valid keys. Valid keys are:\n{AuthProvider.available_providers()}") + f"Credentials dictionary must have valid keys. Valid keys are:\n{AuthProvider.available_providers()}" + ) if not all( key in ["username", "password"] diff --git a/mars-cli/mars_lib/submit.py b/mars-cli/mars_lib/submit.py index 794e551..4e97817 100644 --- a/mars-cli/mars_lib/submit.py +++ b/mars-cli/mars_lib/submit.py @@ -114,7 +114,7 @@ def submission( # Submit to Biosamples biosamples_result = submit_to_biosamples( isa_json=isa_json, - biosamples_credentials=user_credentials[TargetRepository.BIOSAMPLES.value], + biosamples_credentials=user_credentials[AuthProvider.WEBIN.value], biosamples_url=urls["BIOSAMPLES"]["SUBMISSION"], webin_token_url=urls["WEBIN"]["TOKEN"], ) @@ -137,7 +137,7 @@ def submission( file_paths=[ Path(df) for df in data_file_map[TargetRepository.ENA.value] ], - user_credentials=user_credentials[TargetRepository.ENA.value], + user_credentials=user_credentials[AuthProvider.WEBIN.value], submission_url=urls["ENA"]["DATA-SUBMISSION"], file_transfer=file_transfer, ) @@ -148,7 +148,7 @@ def submission( # Step 2 : submit isa-json to ena ena_result = submit_to_ena( isa_json=isa_json, - user_credentials=user_credentials[TargetRepository.ENA.value], + user_credentials=user_credentials[AuthProvider.WEBIN.value], submission_url=urls["ENA"]["SUBMISSION"], ) print_and_log( @@ -173,7 +173,7 @@ def submission( file_transfer=file_transfer, isa_json=isa_json, metabolights_credentials=user_credentials[ - TargetRepository.METABOLIGHTS.value + AuthProvider.METABOLIGHTS_METADATA.value ], metabolights_url=urls["METABOLIGHTS"]["SUBMISSION"], metabolights_token_url=urls["METABOLIGHTS"]["TOKEN"], diff --git a/mars-cli/tests/test_authentication.py b/mars-cli/tests/test_authentication.py index 6957915..8618b21 100644 --- a/mars-cli/tests/test_authentication.py +++ b/mars-cli/tests/test_authentication.py @@ -61,5 +61,7 @@ def test_valid_credentials_file(): _min_credentials = load_credentials("tests/fixtures/min_credentials_file.json") # Test with a credentials file that has an invalid provider - with pytest.raises(ValueError, match="Credentials dictionary must have valid keys."): + with pytest.raises( + ValueError, match="Credentials dictionary must have valid keys." + ): load_credentials("tests/fixtures/bad_credentials_file.json") From 3534f4379355c441092a2242f153cb5bc5761bef Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 2 Jan 2025 12:03:10 +0100 Subject: [PATCH 14/20] Catch supported transfer protocols in CLI --- mars-cli/mars_cli.py | 3 ++- mars-cli/mars_lib/submit.py | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/mars-cli/mars_cli.py b/mars-cli/mars_cli.py index dc23745..24c2893 100644 --- a/mars-cli/mars_cli.py +++ b/mars-cli/mars_cli.py @@ -230,7 +230,8 @@ def cli(ctx, development): @click.option("--submit-to-ena", type=click.BOOL, default=True, help="Submit to ENA.") @click.option( "--file-transfer", - type=click.STRING, + type=click.Choice(["ftp", "aspera"], case_sensitive=False), + required=True, help="provide the name of a file transfer solution, like ftp or aspera", ) @click.option( diff --git a/mars-cli/mars_lib/submit.py b/mars-cli/mars_lib/submit.py index 4e97817..ced441c 100644 --- a/mars-cli/mars_lib/submit.py +++ b/mars-cli/mars_lib/submit.py @@ -387,11 +387,8 @@ def upload_to_ena( submission_url: str, file_transfer: str, ): - ALLOWED_FILE_TRANSFER_SOLUTIONS = {"ftp", "aspera"} file_transfer = file_transfer.lower() - if file_transfer not in ALLOWED_FILE_TRANSFER_SOLUTIONS: - raise ValueError(f"Unsupported transfer protocol: {file_transfer}") if file_transfer == "ftp": uploader = FTPUploader( submission_url, From a394e6df411386d730589ef5772f5d8330fab7a3 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 2 Jan 2025 12:14:04 +0100 Subject: [PATCH 15/20] Make the default ftp --- mars-cli/mars_cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mars-cli/mars_cli.py b/mars-cli/mars_cli.py index 24c2893..196694e 100644 --- a/mars-cli/mars_cli.py +++ b/mars-cli/mars_cli.py @@ -232,7 +232,8 @@ def cli(ctx, development): "--file-transfer", type=click.Choice(["ftp", "aspera"], case_sensitive=False), required=True, - help="provide the name of a file transfer solution, like ftp or aspera", + default="ftp", + help="provide the name of a file transfer solution, like ftp or aspera. The default is ftp.", ) @click.option( "--data-files", From de21fb6edcf97657927362e994aef1bf36dbe69a Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 2 Jan 2025 15:22:35 +0100 Subject: [PATCH 16/20] Remove password from error logs --- mars-cli/mars_lib/submit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mars-cli/mars_lib/submit.py b/mars-cli/mars_lib/submit.py index ced441c..5d24cb5 100644 --- a/mars-cli/mars_lib/submit.py +++ b/mars-cli/mars_lib/submit.py @@ -375,7 +375,7 @@ def submit_to_ena( else result.request.body or "" ) raise requests.HTTPError( - f"Request towards ENA failed!\nRequest:\nMethod:{result.request.method}\nStatus:{result.status_code}\nURL:{result.request.url}\nHeaders:{result.request.headers}\nBody:{body}" + f"Request towards ENA failed!\nRequest:\nMethod:{result.request.method}\nStatus:{result.status_code}\nURL:{submission_url}\nParams: {{'webinUserName': {params.get("webinUserName")}, 'webinPassword': ****}}\nHeaders:{result.request.headers}\nBody:{body}" ) return result From 15b8c6c020998883c8f9c7828ad3f85bd3749e9a Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 2 Jan 2025 16:03:53 +0100 Subject: [PATCH 17/20] Add mypy cache --- .github/workflows/test-mars.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-mars.yml b/.github/workflows/test-mars.yml index e35b4f0..3a74aa8 100644 --- a/.github/workflows/test-mars.yml +++ b/.github/workflows/test-mars.yml @@ -48,6 +48,9 @@ jobs: run: ruff check mars_lib/ working-directory: ${{ env.working-directory }} + - name: Create mypy cache directory + run: mkdir -p /tmp/mypy_cache + - name: Type checking - run: mypy --install-types --non-interactive mars_lib/ + run: mypy --install-types --non-interactive --cache-dir /tmp/mypy_cache mars_lib/ working-directory: ${{ env.working-directory }} From 2f3ac4701483aeb36ae619f5f3034559e27237ce Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 2 Jan 2025 16:07:46 +0100 Subject: [PATCH 18/20] Only test oldest and newest version of python --- .github/workflows/test-mars.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-mars.yml b/.github/workflows/test-mars.yml index 3a74aa8..19015dd 100644 --- a/.github/workflows/test-mars.yml +++ b/.github/workflows/test-mars.yml @@ -15,7 +15,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, windows-latest] - python-version: ["3.9", "3.10", "3.11", "3.12"] + python-version: ["3.9", "3.13"] runs-on: ${{ matrix.os }} env: working-directory: ./mars-cli From 2773709a62401bdcf406c7976d2bcc1fa74f2966 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 2 Jan 2025 16:10:06 +0100 Subject: [PATCH 19/20] Remove braces --- mars-cli/mars_lib/submit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mars-cli/mars_lib/submit.py b/mars-cli/mars_lib/submit.py index 5d24cb5..0326c9d 100644 --- a/mars-cli/mars_lib/submit.py +++ b/mars-cli/mars_lib/submit.py @@ -375,7 +375,7 @@ def submit_to_ena( else result.request.body or "" ) raise requests.HTTPError( - f"Request towards ENA failed!\nRequest:\nMethod:{result.request.method}\nStatus:{result.status_code}\nURL:{submission_url}\nParams: {{'webinUserName': {params.get("webinUserName")}, 'webinPassword': ****}}\nHeaders:{result.request.headers}\nBody:{body}" + f"Request towards ENA failed!\nRequest:\nMethod:{result.request.method}\nStatus:{result.status_code}\nURL:{submission_url}\nParams: ['webinUserName': {params.get("webinUserName")}, 'webinPassword': ****]\nHeaders:{result.request.headers}\nBody:{body}" ) return result From dc51e2d7e3fdd83f50e1440974b18cc7b0ba673d Mon Sep 17 00:00:00 2001 From: bedroesb Date: Thu, 2 Jan 2025 16:42:47 +0100 Subject: [PATCH 20/20] replace double quotes --- mars-cli/mars_lib/submit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mars-cli/mars_lib/submit.py b/mars-cli/mars_lib/submit.py index 0326c9d..11a5063 100644 --- a/mars-cli/mars_lib/submit.py +++ b/mars-cli/mars_lib/submit.py @@ -375,7 +375,7 @@ def submit_to_ena( else result.request.body or "" ) raise requests.HTTPError( - f"Request towards ENA failed!\nRequest:\nMethod:{result.request.method}\nStatus:{result.status_code}\nURL:{submission_url}\nParams: ['webinUserName': {params.get("webinUserName")}, 'webinPassword': ****]\nHeaders:{result.request.headers}\nBody:{body}" + f"Request towards ENA failed!\nRequest:\nMethod:{result.request.method}\nStatus:{result.status_code}\nURL:{submission_url}\nParams: ['webinUserName': {params.get('webinUserName')}, 'webinPassword': ****]\nHeaders:{result.request.headers}\nBody:{body}" ) return result