From 06da32beadb84065c2fbee850e7bdbf878236b96 Mon Sep 17 00:00:00 2001 From: adamrtalbot <12817534+adamrtalbot@users.noreply.github.com> Date: Wed, 26 Feb 2025 12:42:53 +0000 Subject: [PATCH 01/10] First draft of on_exists param --- README.md | 30 +++++++++++-- seqerakit/cli.py | 35 ++++++++++----- seqerakit/helper.py | 14 ++++-- seqerakit/overwrite.py | 49 +++++++++++++++++---- templates/teams.yml | 2 +- tests/unit/test_overwrite.py | 84 ++++++++++++++++++++++++++++++++++++ 6 files changed, 188 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 0f62d4d..341870d 100644 --- a/README.md +++ b/README.md @@ -301,14 +301,34 @@ params: **Note**: If duplicate parameters are provided, the parameters provided as key-value pairs inside the `params` nested dictionary of the YAML file will take precedence **over** values in the provided `params-file`. -### 2. `overwrite` Functionality +### 2. `on_exists` Functionality -For every entity defined in your YAML file, you can specify `overwrite: True` to overwrite any existing entities in Seqera Platform of the same name. +For every entity defined in your YAML file, you can specify how to handle cases where the entity already exists using the `on_exists` parameter with one of three options: -`seqerakit` will first check to see if the name of the entity exists, if so, it will invoke a `tw delete` command before attempting to create it based on the options defined in the YAML file. +- `fail` (default): Raise an error if the entity already exists +- `ignore`: Skip creation if the entity already exists +- `overwrite`: Delete the existing entity and create a new one based on the YAML configuration + +Example usage in YAML: + +```yaml +workspaces: + - name: 'showcase' + organization: 'seqerakit_automation' + on_exists: 'overwrite' # Will delete and recreate if exists +``` + +```yaml +credentials: + - name: 'github_credentials' + workspace: 'seqerakit_automation/showcase' + on_exists: 'ignore' # Will skip if already exists +``` + +When using the `overwrite` option, `seqerakit` will first check if the entity exists, and if so, it will invoke a `tw delete` command before attempting to create it based on the options defined in the YAML file: ```console -DEBUG:root: Overwrite is set to 'True' for organizations +DEBUG:root: on_exists is set to 'overwrite' for organizations DEBUG:root: Running command: tw -o json organizations list DEBUG:root: The attempted organizations resource already exists. Overwriting. @@ -317,6 +337,8 @@ DEBUG:root: Running command: tw organizations delete --name $SEQERA_ORGANIZATION DEBUG:root: Running command: tw organizations add --name $SEQERA_ORGANIZATION_NAME --full-name $SEQERA_ORGANIZATION_NAME --description 'Example of an organization' ``` +> **Note**: For backward compatibility, the `overwrite: True|False` parameter is still supported but deprecated. It will be mapped to `on_exists: 'overwrite'|'fail'` respectively. + ### 3. Specifying JSON configuration files with `file-path` The Seqera Platform CLI allows export and import of entities through JSON configuration files for pipelines and compute environments. To use these files to add a pipeline or compute environment to a workspace, use the `file-path` key to specify a path to a JSON configuration file. diff --git a/seqerakit/cli.py b/seqerakit/cli.py index f125fe8..c1437eb 100644 --- a/seqerakit/cli.py +++ b/seqerakit/cli.py @@ -106,10 +106,18 @@ def parse_args(args=None): type=str, help="Path to a YAML file containing environment variables for configuration.", ) + yaml_processing.add_argument( + "--on-exists", + dest="on_exists", + type=str, + help="Globally specifies the action to take if a resource already exists.", + choices=["fail", "ignore", "overwrite"], + ) yaml_processing.add_argument( "--overwrite", action="store_true", - help="Globally enable overwrite for all resources defined in YAML input(s).", + help="Globally enable overwrite for all resources defined in YAML input(s). ", + deprecated=True, ) return parser.parse_args(args) @@ -147,7 +155,7 @@ def handle_block(self, block, args, destroy=False, dryrun=False): if destroy: logging.debug(" The '--delete' flag has been specified.\n") self.overwrite_method.handle_overwrite( - block, args["cmd_args"], overwrite=False, destroy=True + block, args["cmd_args"], on_exists="fail", destroy=True ) return @@ -162,16 +170,23 @@ def handle_block(self, block, args, destroy=False, dryrun=False): ), } - # Check if overwrite is set to True or globally, and call overwrite handler - overwrite_option = args.get("overwrite", False) or getattr( - self.sp, "overwrite", False - ) - if overwrite_option and not dryrun: - logging.debug(f" Overwrite is set to 'True' for {block}\n") - self.overwrite_method.handle_overwrite( - block, args["cmd_args"], overwrite_option + # Get the on_exists option from args for backward compatibility + on_exists = args.get("on_exists", "fail") + + # For backward compatibility, check if overwrite is set globally + if getattr(self.sp, "overwrite", False): + on_exists = "overwrite" + + if not dryrun: + logging.debug(f" on_exists is set to '{on_exists}' for {block}\n") + should_continue = self.overwrite_method.handle_overwrite( + block, args["cmd_args"], on_exists=on_exists ) + # If on_exists is "ignore" and resource exists, skip creation + if not should_continue: + return + if block in self.list_for_add_method: helper.handle_generic_block(self.sp, block, args["cmd_args"]) elif block in block_handler_map: diff --git a/seqerakit/helper.py b/seqerakit/helper.py index 31c3654..adccbae 100644 --- a/seqerakit/helper.py +++ b/seqerakit/helper.py @@ -163,11 +163,19 @@ def parse_block(block_name, item): } # Use the generic block function as a default. parse_fn = block_to_function.get(block_name, parse_generic_block) - overwrite = item.pop("overwrite", False) - # Call the appropriate function and return its result along with overwrite value. + # Handle both old and new parameters for backward compatibility + overwrite = item.pop("overwrite", None) + on_exists = item.pop("on_exists", "fail") + + # If overwrite is specified, + # it takes precedence over on_exists for backward compatibility + if overwrite is not None: + on_exists = "overwrite" if overwrite else "fail" + + # Call the appropriate function and return its result along with on_exists value. cmd_args = parse_fn(item) - return {"cmd_args": cmd_args, "overwrite": overwrite} + return {"cmd_args": cmd_args, "on_exists": on_exists} # Parsers for certain blocks of yaml that require handling diff --git a/seqerakit/overwrite.py b/seqerakit/overwrite.py index 7cab3d6..931185b 100644 --- a/seqerakit/overwrite.py +++ b/seqerakit/overwrite.py @@ -37,6 +37,9 @@ class Overwrite: "studios", ] + # Define valid on_exists options + VALID_ON_EXISTS_OPTIONS = ["fail", "ignore", "overwrite"] + def __init__(self, sp): """ Initializes an Overwrite instance. @@ -95,11 +98,33 @@ def __init__(self, sp): }, } - def handle_overwrite(self, block, args, overwrite=False, destroy=False): + def handle_overwrite( + self, block, args, on_exists="fail", destroy=False, overwrite=None + ): """ - Handles overwrite functionality for Seqera Platform resources and - calling the 'tw delete' method with the correct args. + Handles resource existence behavior for Seqera Platform resources. + + Args: + block: The resource block type (e.g., "organizations", "teams") + args: Command line arguments for the resource + on_exists: How to handle existing resources. Options: + - "fail" (default): Raise an error if resource exists + - "ignore": Skip creation if resource exists + - "overwrite": Delete existing resource and create new one + destroy: Whether to delete the resource + overwrite: Legacy parameter for backward compatibility """ + # For backward compatibility + if overwrite is not None: + on_exists = "overwrite" if overwrite else "fail" + + # Validate on_exists parameter + if on_exists not in self.VALID_ON_EXISTS_OPTIONS: + raise ValueError( + f"Invalid on_exists option: {on_exists}. " + f"Valid options are: {', '.join(self.VALID_ON_EXISTS_OPTIONS)}" + ) + if block in Overwrite.generic_deletion: self.block_operations[block] = { "keys": ["name", "workspace"], @@ -122,23 +147,31 @@ def handle_overwrite(self, block, args, overwrite=False, destroy=False): elif block == "members": # Rename the user key to name to correctly index JSON data sp_args["name"] = sp_args.pop("user") + if self.check_resource_exists(operation["name_key"], sp_args): - # if resource exists and overwrite is true, delete - if overwrite: + # Handle based on on_exists parameter + if on_exists == "overwrite": logging.info( f" The attempted {block} resource already exists." " Overwriting.\n" ) self.delete_resource(block, operation, sp_args) + elif on_exists == "ignore": + logging.info( + f" The {block} resource already exists." " Skipping creation.\n" + ) + return False elif destroy: logging.info(f" Deleting the {block} resource.") self.delete_resource(block, operation, sp_args) - else: # return an error if resource exists, overwrite=False + else: # fail raise ResourceExistsError( f" The {block} resource already exists and" - " will not be created. Please set 'overwrite: True'" - " in your config file.\n" + " will not be created. Please set 'on_exists: overwrite'" + " or 'on_exists: ignore' in your config file.\n" ) + return True + return True def _get_organization_args(self, args): """ diff --git a/templates/teams.yml b/templates/teams.yml index e6f921b..14da104 100644 --- a/templates/teams.yml +++ b/templates/teams.yml @@ -5,4 +5,4 @@ teams: description: 'My test team' # optional members: # optional - 'my_team_member@gmail.com' - overwrite: True # optional \ No newline at end of file + on_exists: 'overwrite' # optional - values: 'fail' (default), 'ignore', 'overwrite' diff --git a/tests/unit/test_overwrite.py b/tests/unit/test_overwrite.py index 15e83ea..e219449 100644 --- a/tests/unit/test_overwrite.py +++ b/tests/unit/test_overwrite.py @@ -165,6 +165,90 @@ def test_organization_deletion_with_env_var(self, mock_resolve_env_var): self.mock_sp.organizations.assert_called_with("delete", "--name", "${ORG_NAME}") + def test_handle_on_exists_overwrite(self): + # Test for credentials, secrets, compute-envs, datasets, actions, pipelines + args = ["--name", "test-resource", "--workspace", "test-workspace"] + + self.mock_sp.__getattr__("-o json").return_value = json.dumps( + {"name": "test-resource"} + ) + + # Test with on_exists='overwrite' + self.overwrite.handle_overwrite("credentials", args, on_exists="overwrite") + + self.mock_sp.credentials.assert_called_with( + "delete", "--name", "test-resource", "--workspace", "test-workspace" + ) + + def test_handle_on_exists_ignore(self): + # Test for credentials with on_exists='ignore' + args = ["--name", "test-resource", "--workspace", "test-workspace"] + + self.mock_sp.__getattr__("-o json").return_value = json.dumps( + {"name": "test-resource"} + ) + + # Test with on_exists='ignore' + result = self.overwrite.handle_overwrite( + "credentials", args, on_exists="ignore" + ) + + # Should return False to indicate creation should be skipped + self.assertFalse(result) + + # Should not call delete + self.mock_sp.credentials.assert_not_called() + + def test_handle_on_exists_fail(self): + args = ["--name", "test-resource", "--workspace", "test-workspace"] + + # Mock JSON response indicating resource exists + self.mock_sp.__getattr__("-o json").return_value = json.dumps( + {"name": "test-resource"} + ) + + # Test with on_exists='fail' (default) + with self.assertRaises(ResourceExistsError): + self.overwrite.handle_overwrite("credentials", args, on_exists="fail") + + # Should not call delete + self.mock_sp.credentials.assert_not_called() + + def test_handle_invalid_on_exists(self): + args = ["--name", "test-resource", "--workspace", "test-workspace"] + + # Test with invalid on_exists value + with self.assertRaises(ValueError): + self.overwrite.handle_overwrite( + "credentials", args, on_exists="invalid_option" + ) + + def test_backward_compatibility_overwrite(self): + args = ["--name", "test-resource", "--workspace", "test-workspace"] + + self.mock_sp.__getattr__("-o json").return_value = json.dumps( + {"name": "test-resource"} + ) + + # Test with legacy overwrite=True parameter + self.overwrite.handle_overwrite("credentials", args, overwrite=True) + + self.mock_sp.credentials.assert_called_with( + "delete", "--name", "test-resource", "--workspace", "test-workspace" + ) + + def test_backward_compatibility_no_overwrite(self): + args = ["--name", "test-resource", "--workspace", "test-workspace"] + + # Mock JSON response indicating resource exists + self.mock_sp.__getattr__("-o json").return_value = json.dumps( + {"name": "test-resource"} + ) + + # Test with legacy overwrite=False parameter + with self.assertRaises(ResourceExistsError): + self.overwrite.handle_overwrite("credentials", args, overwrite=False) + # TODO: tests for destroy and JSON caching From a722ce1bc6b6b358d1f3bfd88bdfae95f55a5795 Mon Sep 17 00:00:00 2001 From: ejseqera Date: Thu, 23 Jan 2025 23:21:47 -0500 Subject: [PATCH 02/10] ci: fix revision used for rnaseq in tower actions --- examples/yaml/e2e/actions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/yaml/e2e/actions.yml b/examples/yaml/e2e/actions.yml index 95b7533..78620fe 100644 --- a/examples/yaml/e2e/actions.yml +++ b/examples/yaml/e2e/actions.yml @@ -5,7 +5,7 @@ actions: workspace: 'seqerakit-e2e/showcase' compute-env: 'seqera_aws_virginia_fusionv2_nvme' work-dir: 's3://scidev-testing' - revision: 'main' + revision: 'master' profile: 'test' params: outdir: s3://seqeralabs-showcase/nf-core-rnaseq/action/' From 3ad55adc25db4d431881b186a73407e2155845d0 Mon Sep 17 00:00:00 2001 From: adamrtalbot <12817534+adamrtalbot@users.noreply.github.com> Date: Wed, 26 Feb 2025 14:58:47 +0000 Subject: [PATCH 03/10] fixup --- examples/yaml/e2e/actions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/yaml/e2e/actions.yml b/examples/yaml/e2e/actions.yml index 78620fe..95b7533 100644 --- a/examples/yaml/e2e/actions.yml +++ b/examples/yaml/e2e/actions.yml @@ -5,7 +5,7 @@ actions: workspace: 'seqerakit-e2e/showcase' compute-env: 'seqera_aws_virginia_fusionv2_nvme' work-dir: 's3://scidev-testing' - revision: 'master' + revision: 'main' profile: 'test' params: outdir: s3://seqeralabs-showcase/nf-core-rnaseq/action/' From 27ebf747a2f214cb4142e17ff95ca1533100ac48 Mon Sep 17 00:00:00 2001 From: adamrtalbot <12817534+adamrtalbot@users.noreply.github.com> Date: Wed, 26 Feb 2025 15:41:28 +0000 Subject: [PATCH 04/10] Fix CLI and add deprecation warning --- seqerakit/cli.py | 20 ++++++++++++++++---- seqerakit/overwrite.py | 5 +++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/seqerakit/cli.py b/seqerakit/cli.py index c1437eb..98cef3f 100644 --- a/seqerakit/cli.py +++ b/seqerakit/cli.py @@ -21,7 +21,7 @@ import logging import sys import os -import yaml +import yaml # type: ignore from pathlib import Path @@ -116,7 +116,9 @@ def parse_args(args=None): yaml_processing.add_argument( "--overwrite", action="store_true", - help="Globally enable overwrite for all resources defined in YAML input(s). ", + help=""" + Globally enable overwrite for all resources defined in YAML input(s). + "Deprecated: Please use '--on-exists=overwrite' instead.""", deprecated=True, ) return parser.parse_args(args) @@ -173,8 +175,15 @@ def handle_block(self, block, args, destroy=False, dryrun=False): # Get the on_exists option from args for backward compatibility on_exists = args.get("on_exists", "fail") - # For backward compatibility, check if overwrite is set globally - if getattr(self.sp, "overwrite", False): + # Global settings take precedence over block-level settings + # First check the global --on-exists parameter + if ( + hasattr(self.sp, "global_on_exists") + and self.sp.global_on_exists is not None + ): + on_exists = self.sp.global_on_exists + # Then check for the deprecated global --overwrite flag + elif getattr(self.sp, "overwrite", False): on_exists = "overwrite" if not dryrun: @@ -263,6 +272,9 @@ def main(args=None): ) sp.overwrite = options.overwrite # If global overwrite is set + # Store the global on_exists parameter if provided + sp.global_on_exists = options.on_exists if options.on_exists else None + # If the info flag is set, run 'tw info' try: if options.info: diff --git a/seqerakit/overwrite.py b/seqerakit/overwrite.py index 931185b..ff35ed1 100644 --- a/seqerakit/overwrite.py +++ b/seqerakit/overwrite.py @@ -167,8 +167,9 @@ def handle_overwrite( else: # fail raise ResourceExistsError( f" The {block} resource already exists and" - " will not be created. Please set 'on_exists: overwrite'" - " or 'on_exists: ignore' in your config file.\n" + " will not be created. Please set 'on_exists: overwrite' " + " to replace the resource or set 'on_exists: ignore' to " + " ignore this error.\n" ) return True return True From b2804d076a36f5839a88a02c0b16746197bbac36 Mon Sep 17 00:00:00 2001 From: adamrtalbot <12817534+adamrtalbot@users.noreply.github.com> Date: Wed, 26 Feb 2025 18:26:40 +0000 Subject: [PATCH 05/10] fix tests --- seqerakit/helper.py | 12 ++++++++++- tests/unit/test_helper.py | 42 +++++++++++++++++++-------------------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/seqerakit/helper.py b/seqerakit/helper.py index adccbae..8344849 100644 --- a/seqerakit/helper.py +++ b/seqerakit/helper.py @@ -175,7 +175,17 @@ def parse_block(block_name, item): # Call the appropriate function and return its result along with on_exists value. cmd_args = parse_fn(item) - return {"cmd_args": cmd_args, "on_exists": on_exists} + + # Return both on_exists and overwrite for backward compatibility + # This allows existing tests to continue using overwrite key + # while new code can use the on_exists key + result = {"cmd_args": cmd_args, "on_exists": on_exists} + + # Set overwrite boolean for test compatibility + # If on_exists is 'overwrite', set overwrite to True, otherwise False + result["overwrite"] = on_exists == "overwrite" + + return result # Parsers for certain blocks of yaml that require handling diff --git a/tests/unit/test_helper.py b/tests/unit/test_helper.py index 4247b0d..40b4c19 100644 --- a/tests/unit/test_helper.py +++ b/tests/unit/test_helper.py @@ -28,7 +28,7 @@ def test_create_mock_organization_yaml(mock_yaml_file): "description": "My test organization 1", "location": "Global", "url": "https://example.com", - "overwrite": True, + "on_exists": "overwrite", } ] } @@ -46,7 +46,7 @@ def test_create_mock_organization_yaml(mock_yaml_file): "--url", "https://example.com", ], - "overwrite": True, + "on_exists": "overwrite", } ] file_path = mock_yaml_file(test_data) @@ -66,7 +66,7 @@ def test_create_mock_workspace_yaml(mock_yaml_file): "organization": "my_organization", "description": "My test workspace 1", "visibility": "PRIVATE", - "overwrite": True, + "on_exists": "overwrite", } ] } @@ -84,7 +84,7 @@ def test_create_mock_workspace_yaml(mock_yaml_file): "--visibility", "PRIVATE", ], - "overwrite": True, + "on_exists": "overwrite", } ] @@ -104,7 +104,7 @@ def test_create_mock_dataset_yaml(mock_yaml_file): "workspace": "my_organization/my_workspace", "header": True, "file-path": "./examples/yaml/datasets/samples.csv", - "overwrite": True, + "on_exists": "overwrite", } ] } @@ -120,7 +120,7 @@ def test_create_mock_dataset_yaml(mock_yaml_file): "My test dataset 1", "--header", ], - "overwrite": True, + "on_exists": "overwrite", } ] @@ -142,7 +142,7 @@ def test_create_mock_computeevs_source_yaml(mock_yaml_file): "wait": "AVAILABLE", "fusion-v2": True, "fargate": False, - "overwrite": True, + "on_exists": "overwrite", } ], } @@ -161,7 +161,7 @@ def test_create_mock_computeevs_source_yaml(mock_yaml_file): "--workspace", "my_organization/my_workspace", ], - "overwrite": True, + "on_exists": "overwrite", } ] @@ -200,7 +200,7 @@ def test_create_mock_computeevs_cli_yaml(mock_yaml_file): "--workspace", "my_organization/my_workspace", ], - "overwrite": False, + "on_exists": "fail", } ] file_path = mock_yaml_file(test_data) @@ -224,7 +224,7 @@ def test_create_mock_pipeline_add_yaml(mock_yaml_file): "config": "./examples/yaml/pipelines/test_pipeline1/config.txt", "pre-run": "./examples/yaml/pipelines/test_pipeline1/pre_run.sh", "revision": "master", - "overwrite": True, + "on_exists": "overwrite", "stub-run": True, } ] @@ -256,7 +256,7 @@ def test_create_mock_pipeline_add_yaml(mock_yaml_file): "--params-file", "./examples/yaml/pipelines/test_pipeline1/params.yaml", ], - "overwrite": True, + "on_exists": "overwrite", } ] @@ -275,7 +275,7 @@ def test_create_mock_teams_yaml(mock_yaml_file): "organization": "my_organization", "description": "My test team 1", "members": ["user1@org.io"], - "overwrite": True, + "on_exists": "overwrite", }, ] } @@ -302,7 +302,7 @@ def test_create_mock_teams_yaml(mock_yaml_file): ] ], ), - "overwrite": True, + "on_exists": "overwrite", } ] @@ -323,7 +323,7 @@ def test_create_mock_members_yaml(mock_yaml_file): "--user", "bob@myorg.io", ], - "overwrite": False, + "on_exists": "fail", } ] file_path = mock_yaml_file(test_data) @@ -344,7 +344,7 @@ def test_create_mock_studios_yaml(mock_yaml_file): "cpu": 2, "memory": 4096, "autoStart": False, - "overwrite": True, + "on_exists": "overwrite", "mount-data-ids": "v1-user-bf73f9d33997f93a20ee3e6911779951", } ] @@ -368,7 +368,7 @@ def test_create_mock_studios_yaml(mock_yaml_file): "--workspace", "my_organization/my_workspace", ], - "overwrite": True, + "on_exists": "overwrite", } ] @@ -388,7 +388,7 @@ def test_create_mock_data_links_yaml(mock_yaml_file): "provider": "aws", "credentials": "my_credentials", "uri": "s3://scidev-playground-eu-west-2/esha/nf-core-scrnaseq/", - "overwrite": True, + "on_exists": "overwrite", } ] } @@ -406,7 +406,7 @@ def test_create_mock_data_links_yaml(mock_yaml_file): "--workspace", "my_organization/my_workspace", ], - "overwrite": True, + "on_exists": "overwrite", } ] file_path = mock_yaml_file(test_data) @@ -466,7 +466,7 @@ def test_stdin_yaml_file(): "--wait", "AVAILABLE", ], - "overwrite": False, + "on_exists": "fail", } ] assert "compute-envs" in result @@ -549,7 +549,7 @@ def test_targets_specified(): expected_organizations_output = [ { "cmd_args": ["--name", "org1", "--description", "Organization 1"], - "overwrite": False, + "on_exists": "fail", } ] expected_workspaces_output = [ @@ -562,7 +562,7 @@ def test_targets_specified(): "--description", "Workspace 1", ], - "overwrite": False, + "on_exists": "fail", } ] # Check that only 'organizations' and 'workspaces' are in the result From 666aaaf3a751b6c39863f52a55f38c43389a07ea Mon Sep 17 00:00:00 2001 From: adamrtalbot <12817534+adamrtalbot@users.noreply.github.com> Date: Sun, 2 Mar 2025 11:42:22 +0000 Subject: [PATCH 06/10] Use enum to strictly type the on_exists behaviour --- seqerakit/cli.py | 26 +++++++++++++++------ seqerakit/helper.py | 25 ++++++++++++++------ seqerakit/overwrite.py | 44 ++++++++++++++++++++++-------------- tests/unit/test_overwrite.py | 23 ++++++++++++++----- 4 files changed, 81 insertions(+), 37 deletions(-) diff --git a/seqerakit/cli.py b/seqerakit/cli.py index 98cef3f..cb51047 100644 --- a/seqerakit/cli.py +++ b/seqerakit/cli.py @@ -32,13 +32,14 @@ CommandError, ) from seqerakit import __version__ +from seqerakit.on_exists import OnExists logger = logging.getLogger(__name__) def parse_args(args=None): parser = argparse.ArgumentParser( - description="Seqerakit: Python wrapper for the Seqera Platform CLI" + description="Build a Seqera Platform instance from a YAML configuration file." ) # General options general = parser.add_argument_group("General Options") @@ -111,7 +112,7 @@ def parse_args(args=None): dest="on_exists", type=str, help="Globally specifies the action to take if a resource already exists.", - choices=["fail", "ignore", "overwrite"], + choices=[e.name.lower() for e in OnExists], ) yaml_processing.add_argument( "--overwrite", @@ -157,7 +158,7 @@ def handle_block(self, block, args, destroy=False, dryrun=False): if destroy: logging.debug(" The '--delete' flag has been specified.\n") self.overwrite_method.handle_overwrite( - block, args["cmd_args"], on_exists="fail", destroy=True + block, args["cmd_args"], on_exists=OnExists.FAIL, destroy=True ) return @@ -173,7 +174,7 @@ def handle_block(self, block, args, destroy=False, dryrun=False): } # Get the on_exists option from args for backward compatibility - on_exists = args.get("on_exists", "fail") + on_exists = args.get("on_exists", OnExists.FAIL) # Global settings take precedence over block-level settings # First check the global --on-exists parameter @@ -184,10 +185,12 @@ def handle_block(self, block, args, destroy=False, dryrun=False): on_exists = self.sp.global_on_exists # Then check for the deprecated global --overwrite flag elif getattr(self.sp, "overwrite", False): - on_exists = "overwrite" + on_exists = OnExists.OVERWRITE if not dryrun: - logging.debug(f" on_exists is set to '{on_exists}' for {block}\n") + logging.debug( + f" on_exists is set to '{on_exists.name.lower()}' for {block}\n" + ) should_continue = self.overwrite_method.handle_overwrite( block, args["cmd_args"], on_exists=on_exists ) @@ -273,7 +276,16 @@ def main(args=None): sp.overwrite = options.overwrite # If global overwrite is set # Store the global on_exists parameter if provided - sp.global_on_exists = options.on_exists if options.on_exists else None + if options.on_exists: + try: + sp.global_on_exists = OnExists[options.on_exists.upper()] + except KeyError as err: + logging.error(f"Invalid on_exists option: {options.on_exists}") + raise err + else: + sp.global_on_exists = ( + None # Don't set a default, let the block-level or overwrite flag handle it + ) # If the info flag is set, run 'tw info' try: diff --git a/seqerakit/helper.py b/seqerakit/helper.py index 8344849..f6a9f68 100644 --- a/seqerakit/helper.py +++ b/seqerakit/helper.py @@ -17,10 +17,11 @@ Including handling methods for each block in the YAML file, and parsing methods for each block in the YAML file. """ -import yaml +import yaml # type: ignore from seqerakit import utils import sys import json +from seqerakit.on_exists import OnExists def parse_yaml_block(yaml_data, block_name): @@ -166,24 +167,34 @@ def parse_block(block_name, item): # Handle both old and new parameters for backward compatibility overwrite = item.pop("overwrite", None) - on_exists = item.pop("on_exists", "fail") + on_exists_str = item.pop("on_exists", "fail") + + # Convert string to enum + if isinstance(on_exists_str, str): + on_exists_str = on_exists_str.upper() + try: + on_exists = OnExists[on_exists_str] + except KeyError: + # Default to FAIL for invalid values + on_exists = OnExists.FAIL + else: + # If it's already an enum, use it directly + on_exists = on_exists_str # If overwrite is specified, # it takes precedence over on_exists for backward compatibility if overwrite is not None: - on_exists = "overwrite" if overwrite else "fail" + on_exists = OnExists.OVERWRITE if overwrite else OnExists.FAIL # Call the appropriate function and return its result along with on_exists value. cmd_args = parse_fn(item) # Return both on_exists and overwrite for backward compatibility - # This allows existing tests to continue using overwrite key - # while new code can use the on_exists key - result = {"cmd_args": cmd_args, "on_exists": on_exists} + result = {"cmd_args": cmd_args, "on_exists": on_exists.name.lower()} # Set overwrite boolean for test compatibility # If on_exists is 'overwrite', set overwrite to True, otherwise False - result["overwrite"] = on_exists == "overwrite" + result["overwrite"] = on_exists == OnExists.OVERWRITE return result diff --git a/seqerakit/overwrite.py b/seqerakit/overwrite.py index ff35ed1..7260942 100644 --- a/seqerakit/overwrite.py +++ b/seqerakit/overwrite.py @@ -16,6 +16,7 @@ from seqerakit import utils from seqerakit.seqeraplatform import ResourceExistsError import logging +from seqerakit.on_exists import OnExists class Overwrite: @@ -37,8 +38,8 @@ class Overwrite: "studios", ] - # Define valid on_exists options - VALID_ON_EXISTS_OPTIONS = ["fail", "ignore", "overwrite"] + # Define valid on_exists options as enum values + VALID_ON_EXISTS_OPTIONS = [e.name.lower() for e in OnExists] def __init__(self, sp): """ @@ -48,10 +49,9 @@ def __init__(self, sp): sp: A SeqeraPlatform class instance. Attributes: - sp: A SeqeraPlatform class instance used to execute CLI commands. - cached_jsondata: A cached placeholder for JSON data. Default value is None. - block_jsondata: A dictionary to store JSON data for each block. - Key is the block name, and value is the corresponding JSON data. + sp: A SeqeraPlatform class instance. + block_jsondata: A dictionary to cache JSON data for resources. + block_operations: A dictionary mapping resource types to their operations. """ self.sp = sp self.cached_jsondata = None @@ -99,7 +99,7 @@ def __init__(self, sp): } def handle_overwrite( - self, block, args, on_exists="fail", destroy=False, overwrite=None + self, block, args, on_exists=OnExists.FAIL, destroy=False, overwrite=None ): """ Handles resource existence behavior for Seqera Platform resources. @@ -108,20 +108,31 @@ def handle_overwrite( block: The resource block type (e.g., "organizations", "teams") args: Command line arguments for the resource on_exists: How to handle existing resources. Options: - - "fail" (default): Raise an error if resource exists - - "ignore": Skip creation if resource exists - - "overwrite": Delete existing resource and create new one + - OnExists.FAIL (default): Raise an error if resource exists + - OnExists.IGNORE: Skip creation if resource exists + - OnExists.OVERWRITE: Delete existing resource and create new one destroy: Whether to delete the resource overwrite: Legacy parameter for backward compatibility """ + # Convert string to enum if needed + if isinstance(on_exists, str): + on_exists_str = on_exists.upper() + try: + on_exists = OnExists[on_exists_str] + except KeyError: + raise ValueError( + f"Invalid on_exists option: {on_exists}. " + f"Valid options are: {', '.join(self.VALID_ON_EXISTS_OPTIONS)}" + ) + # For backward compatibility if overwrite is not None: - on_exists = "overwrite" if overwrite else "fail" + on_exists = OnExists.OVERWRITE if overwrite else OnExists.FAIL - # Validate on_exists parameter - if on_exists not in self.VALID_ON_EXISTS_OPTIONS: + # Validate on_exists parameter (should always be valid if it's an enum) + if on_exists.name.lower() not in self.VALID_ON_EXISTS_OPTIONS: raise ValueError( - f"Invalid on_exists option: {on_exists}. " + f"Invalid on_exists option: {on_exists_str}. " f"Valid options are: {', '.join(self.VALID_ON_EXISTS_OPTIONS)}" ) @@ -150,13 +161,13 @@ def handle_overwrite( if self.check_resource_exists(operation["name_key"], sp_args): # Handle based on on_exists parameter - if on_exists == "overwrite": + if on_exists == OnExists.OVERWRITE: logging.info( f" The attempted {block} resource already exists." " Overwriting.\n" ) self.delete_resource(block, operation, sp_args) - elif on_exists == "ignore": + elif on_exists == OnExists.IGNORE: logging.info( f" The {block} resource already exists." " Skipping creation.\n" ) @@ -315,7 +326,6 @@ def _get_json_data(self, block, args, keys_to_get): with self.sp.suppress_output(): self.cached_jsondata = json_method(block, "list") - self.block_jsondata[block] = self.cached_jsondata return self.cached_jsondata, sp_args def check_resource_exists(self, name_key, sp_args): diff --git a/tests/unit/test_overwrite.py b/tests/unit/test_overwrite.py index e219449..e4abd60 100644 --- a/tests/unit/test_overwrite.py +++ b/tests/unit/test_overwrite.py @@ -3,6 +3,7 @@ import json from seqerakit.overwrite import Overwrite from seqerakit.seqeraplatform import ResourceExistsError +from seqerakit.on_exists import OnExists class TestOverwrite(unittest.TestCase): @@ -43,7 +44,9 @@ def test_handle_overwrite_generic_deletion(self): {"name": "test-resource"} ) - self.overwrite.handle_overwrite("credentials", args, overwrite=True) + self.overwrite.handle_overwrite( + "credentials", args, on_exists=OnExists.OVERWRITE + ) self.mock_sp.credentials.assert_called_with( "delete", "--name", "test-resource", "--workspace", "test-workspace" @@ -58,7 +61,9 @@ def test_handle_overwrite_resource_exists_no_overwrite(self): ) with self.assertRaises(ResourceExistsError): - self.overwrite.handle_overwrite("credentials", args, overwrite=False) + self.overwrite.handle_overwrite( + "credentials", args, on_exists=OnExists.FAIL + ) def test_team_deletion(self): args = {"name": "test-team", "organization": "test-org"} @@ -92,7 +97,9 @@ def test_workspace_deletion(self): self.mock_sp.__getattr__("-o json").return_value = self.sample_workspace_json - self.overwrite.handle_overwrite("workspaces", args, overwrite=True) + self.overwrite.handle_overwrite( + "workspaces", args, on_exists=OnExists.OVERWRITE + ) self.mock_sp.workspaces.assert_called_with("delete", "--id", "456") @@ -108,7 +115,7 @@ def test_label_deletion(self): self.mock_sp.__getattr__("-o json").return_value = self.sample_labels_json - self.overwrite.handle_overwrite("labels", args, overwrite=True) + self.overwrite.handle_overwrite("labels", args, on_exists=OnExists.OVERWRITE) self.mock_sp.labels.assert_called_with( "delete", "--id", "789", "-w", "test-workspace" @@ -128,7 +135,9 @@ def test_participant_deletion(self): {"teamName": "test-team"} ) - self.overwrite.handle_overwrite("participants", team_args, overwrite=True) + self.overwrite.handle_overwrite( + "participants", team_args, on_exists=OnExists.OVERWRITE + ) self.mock_sp.participants.assert_called_with( "delete", @@ -159,7 +168,9 @@ def test_organization_deletion_with_env_var(self, mock_resolve_env_var): self.mock_sp.configure_mock(**{"-o json": json_method_mock}) - self.overwrite.handle_overwrite("organizations", args, overwrite=True) + self.overwrite.handle_overwrite( + "organizations", args, on_exists=OnExists.OVERWRITE + ) mock_resolve_env_var.assert_any_call("${ORG_NAME}") From 4c898f0672be2134df2c733502c72a05d5ae89cf Mon Sep 17 00:00:00 2001 From: adamrtalbot <12817534+adamrtalbot@users.noreply.github.com> Date: Sun, 2 Mar 2025 11:46:04 +0000 Subject: [PATCH 07/10] fixup --- seqerakit/on_exists.py | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 seqerakit/on_exists.py diff --git a/seqerakit/on_exists.py b/seqerakit/on_exists.py new file mode 100644 index 0000000..d89ceee --- /dev/null +++ b/seqerakit/on_exists.py @@ -0,0 +1,9 @@ +from enum import Enum, auto + + +class OnExists(Enum): + """Enum defining behavior when a resource already exists.""" + + FAIL = auto() + IGNORE = auto() + OVERWRITE = auto() From fc1821b456518bac54f1a070d3fb8bf694a78786 Mon Sep 17 00:00:00 2001 From: adamrtalbot <12817534+adamrtalbot@users.noreply.github.com> Date: Sun, 2 Mar 2025 12:15:26 +0000 Subject: [PATCH 08/10] test fix --- seqerakit/helper.py | 12 +++--------- seqerakit/overwrite.py | 10 ++++++---- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/seqerakit/helper.py b/seqerakit/helper.py index f6a9f68..b4b2697 100644 --- a/seqerakit/helper.py +++ b/seqerakit/helper.py @@ -169,7 +169,7 @@ def parse_block(block_name, item): overwrite = item.pop("overwrite", None) on_exists_str = item.pop("on_exists", "fail") - # Convert string to enum + # Convert string to enum if needed if isinstance(on_exists_str, str): on_exists_str = on_exists_str.upper() try: @@ -189,14 +189,8 @@ def parse_block(block_name, item): # Call the appropriate function and return its result along with on_exists value. cmd_args = parse_fn(item) - # Return both on_exists and overwrite for backward compatibility - result = {"cmd_args": cmd_args, "on_exists": on_exists.name.lower()} - - # Set overwrite boolean for test compatibility - # If on_exists is 'overwrite', set overwrite to True, otherwise False - result["overwrite"] = on_exists == OnExists.OVERWRITE - - return result + # Return only the string value of on_exists for test compatibility + return {"cmd_args": cmd_args, "on_exists": on_exists.name.lower()} # Parsers for certain blocks of yaml that require handling diff --git a/seqerakit/overwrite.py b/seqerakit/overwrite.py index 7260942..aea97e4 100644 --- a/seqerakit/overwrite.py +++ b/seqerakit/overwrite.py @@ -15,8 +15,8 @@ import json from seqerakit import utils from seqerakit.seqeraplatform import ResourceExistsError -import logging from seqerakit.on_exists import OnExists +import logging class Overwrite: @@ -49,9 +49,10 @@ def __init__(self, sp): sp: A SeqeraPlatform class instance. Attributes: - sp: A SeqeraPlatform class instance. - block_jsondata: A dictionary to cache JSON data for resources. - block_operations: A dictionary mapping resource types to their operations. + sp: A SeqeraPlatform class instance used to execute CLI commands. + cached_jsondata: A cached placeholder for JSON data. Default value is None. + block_jsondata: A dictionary to store JSON data for each block. + Key is the block name, and value is the corresponding JSON data. """ self.sp = sp self.cached_jsondata = None @@ -326,6 +327,7 @@ def _get_json_data(self, block, args, keys_to_get): with self.sp.suppress_output(): self.cached_jsondata = json_method(block, "list") + self.block_jsondata[block] = self.cached_jsondata return self.cached_jsondata, sp_args def check_resource_exists(self, name_key, sp_args): From b6cb4dae9d3aafe9cb6cc631d81a0727cbea8e33 Mon Sep 17 00:00:00 2001 From: adamrtalbot <12817534+adamrtalbot@users.noreply.github.com> Date: Sun, 2 Mar 2025 12:20:00 +0000 Subject: [PATCH 09/10] more fixy --- seqerakit/cli.py | 13 ++++++++++--- seqerakit/overwrite.py | 8 ++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/seqerakit/cli.py b/seqerakit/cli.py index cb51047..923b7a2 100644 --- a/seqerakit/cli.py +++ b/seqerakit/cli.py @@ -174,7 +174,12 @@ def handle_block(self, block, args, destroy=False, dryrun=False): } # Get the on_exists option from args for backward compatibility - on_exists = args.get("on_exists", OnExists.FAIL) + on_exists = args.get("on_exists", "fail") + if isinstance(on_exists, str): + try: + on_exists = OnExists[on_exists.upper()] + except KeyError: + on_exists = OnExists.FAIL # Global settings take precedence over block-level settings # First check the global --on-exists parameter @@ -188,9 +193,11 @@ def handle_block(self, block, args, destroy=False, dryrun=False): on_exists = OnExists.OVERWRITE if not dryrun: - logging.debug( - f" on_exists is set to '{on_exists.name.lower()}' for {block}\n" + # Use on_exists.name.lower() only if it's an enum, otherwise use the string + on_exists_str = ( + on_exists.name.lower() if hasattr(on_exists, "name") else on_exists ) + logging.debug(f" on_exists is set to '{on_exists_str}' for {block}\n") should_continue = self.overwrite_method.handle_overwrite( block, args["cmd_args"], on_exists=on_exists ) diff --git a/seqerakit/overwrite.py b/seqerakit/overwrite.py index aea97e4..b820363 100644 --- a/seqerakit/overwrite.py +++ b/seqerakit/overwrite.py @@ -178,10 +178,10 @@ def handle_overwrite( self.delete_resource(block, operation, sp_args) else: # fail raise ResourceExistsError( - f" The {block} resource already exists and" - " will not be created. Please set 'on_exists: overwrite' " - " to replace the resource or set 'on_exists: ignore' to " - " ignore this error.\n" + f"The {block} resource already exists and " + "will not be created. Please set 'on_exists: overwrite' " + "to replace the resource or set 'on_exists: ignore' to " + "ignore this error.\n" ) return True return True From 7f6786b5046b6bb190a3bd82f4611a6b592510ec Mon Sep 17 00:00:00 2001 From: adamrtalbot <12817534+adamrtalbot@users.noreply.github.com> Date: Mon, 3 Mar 2025 10:48:15 +0000 Subject: [PATCH 10/10] code tidy --- seqerakit/cli.py | 39 ++++++++++++++++++++++----------------- seqerakit/helper.py | 30 ++++++++++++++---------------- seqerakit/overwrite.py | 10 +--------- 3 files changed, 37 insertions(+), 42 deletions(-) diff --git a/seqerakit/cli.py b/seqerakit/cli.py index 923b7a2..bded549 100644 --- a/seqerakit/cli.py +++ b/seqerakit/cli.py @@ -173,25 +173,32 @@ def handle_block(self, block, args, destroy=False, dryrun=False): ), } - # Get the on_exists option from args for backward compatibility - on_exists = args.get("on_exists", "fail") - if isinstance(on_exists, str): - try: - on_exists = OnExists[on_exists.upper()] - except KeyError: - on_exists = OnExists.FAIL - - # Global settings take precedence over block-level settings - # First check the global --on-exists parameter + # Determine the on_exists behavior (default to FAIL) + on_exists = OnExists.FAIL + + # Check for global settings (they override block-level settings) if ( hasattr(self.sp, "global_on_exists") and self.sp.global_on_exists is not None ): on_exists = self.sp.global_on_exists - # Then check for the deprecated global --overwrite flag elif getattr(self.sp, "overwrite", False): + logging.warning( + "The '--overwrite' flag is deprecated. " + "Please use '--on-exists=overwrite' instead." + ) on_exists = OnExists.OVERWRITE + # If no global setting, use block-level setting if provided + elif "on_exists" in args: + on_exists_value = args["on_exists"] + if isinstance(on_exists_value, str): + try: + on_exists = OnExists[on_exists_value.upper()] + except KeyError as err: + logging.error(f"Invalid on_exists option: {on_exists_value}") + raise err + if not dryrun: # Use on_exists.name.lower() only if it's an enum, otherwise use the string on_exists_str = ( @@ -282,17 +289,15 @@ def main(args=None): ) sp.overwrite = options.overwrite # If global overwrite is set - # Store the global on_exists parameter if provided + # Set global on_exists parameter if provided if options.on_exists: try: sp.global_on_exists = OnExists[options.on_exists.upper()] - except KeyError as err: + except KeyError: logging.error(f"Invalid on_exists option: {options.on_exists}") - raise err + raise else: - sp.global_on_exists = ( - None # Don't set a default, let the block-level or overwrite flag handle it - ) + sp.global_on_exists = None # If the info flag is set, run 'tw info' try: diff --git a/seqerakit/helper.py b/seqerakit/helper.py index b4b2697..d417310 100644 --- a/seqerakit/helper.py +++ b/seqerakit/helper.py @@ -165,31 +165,29 @@ def parse_block(block_name, item): # Use the generic block function as a default. parse_fn = block_to_function.get(block_name, parse_generic_block) - # Handle both old and new parameters for backward compatibility + # Get on_exists setting with backward compatibility for overwrite overwrite = item.pop("overwrite", None) on_exists_str = item.pop("on_exists", "fail") - # Convert string to enum if needed - if isinstance(on_exists_str, str): - on_exists_str = on_exists_str.upper() + # Determine final on_exists value + if overwrite is not None: + # overwrite takes precedence for backward compatibility + on_exists = OnExists.OVERWRITE if overwrite else OnExists.FAIL + elif isinstance(on_exists_str, str): try: - on_exists = OnExists[on_exists_str] + on_exists = OnExists[on_exists_str.upper()] except KeyError: - # Default to FAIL for invalid values - on_exists = OnExists.FAIL + raise ValueError( + f"Invalid on_exists option: '{on_exists_str}'. " + f"Valid options are: " + f"{', '.join(behaviour.name.lower() for behaviour in OnExists)}" + ) else: - # If it's already an enum, use it directly + # Use directly if already an enum on_exists = on_exists_str - # If overwrite is specified, - # it takes precedence over on_exists for backward compatibility - if overwrite is not None: - on_exists = OnExists.OVERWRITE if overwrite else OnExists.FAIL - - # Call the appropriate function and return its result along with on_exists value. + # Parse the block and return with on_exists value cmd_args = parse_fn(item) - - # Return only the string value of on_exists for test compatibility return {"cmd_args": cmd_args, "on_exists": on_exists.name.lower()} diff --git a/seqerakit/overwrite.py b/seqerakit/overwrite.py index b820363..48af5b7 100644 --- a/seqerakit/overwrite.py +++ b/seqerakit/overwrite.py @@ -117,9 +117,8 @@ def handle_overwrite( """ # Convert string to enum if needed if isinstance(on_exists, str): - on_exists_str = on_exists.upper() try: - on_exists = OnExists[on_exists_str] + on_exists = OnExists[on_exists.upper()] except KeyError: raise ValueError( f"Invalid on_exists option: {on_exists}. " @@ -130,13 +129,6 @@ def handle_overwrite( if overwrite is not None: on_exists = OnExists.OVERWRITE if overwrite else OnExists.FAIL - # Validate on_exists parameter (should always be valid if it's an enum) - if on_exists.name.lower() not in self.VALID_ON_EXISTS_OPTIONS: - raise ValueError( - f"Invalid on_exists option: {on_exists_str}. " - f"Valid options are: {', '.join(self.VALID_ON_EXISTS_OPTIONS)}" - ) - if block in Overwrite.generic_deletion: self.block_operations[block] = { "keys": ["name", "workspace"],