From 29347b37b552424cc444b51d09ccdf8bf4414f27 Mon Sep 17 00:00:00 2001 From: "Juan E. Arango Ossa" Date: Tue, 24 Oct 2023 17:13:53 -0400 Subject: [PATCH 1/3] =?UTF-8?q?=E2=9D=8C=20add=20REJECTED=20choice=20and?= =?UTF-8?q?=20cli=20to=20reject=20analyses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- isabl_cli/app.py | 4 ++-- isabl_cli/commands.py | 10 ++++++++++ isabl_cli/options.py | 1 + tests/test_commands.py | 22 +++++++++++++++++++--- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/isabl_cli/app.py b/isabl_cli/app.py index 4b724ea..fbd8991 100644 --- a/isabl_cli/app.py +++ b/isabl_cli/app.py @@ -99,7 +99,7 @@ class AbstractApplication: # pylint: disable=too-many-public-methods # Analyses in these status won't be prepared for submission. To re-rerun SUCCEEDED # analyses see unique_analysis_per_individual. To re-rerun failed analyses use # either --force or --restart. - skip_status = {"FAILED", "FINISHED", "STARTED", "SUBMITTED", "SUCCEEDED"} + skip_status = {"FAILED", "FINISHED", "STARTED", "SUBMITTED", "SUCCEEDED", "REJECTED"} # If any of these errors is raised during the command generation process, the # submission will continue. Errors or valdation messages are presented at the end. @@ -957,7 +957,7 @@ def run_analyses(self, analyses, commit, force, restart, local): api.patch_analysis_status(i, "STAGED") # trash analysis and create outdir again - elif force and i["status"] not in {"SUCCEEDED", "FINISHED"}: + elif force and i["status"] not in {"SUCCEEDED", "FINISHED", "REJECTED"}: system_settings.TRASH_ANALYSIS_STORAGE(i) utils.makedirs(i["storage_url"]) diff --git a/isabl_cli/commands.py b/isabl_cli/commands.py index 6a56d91..2694140 100644 --- a/isabl_cli/commands.py +++ b/isabl_cli/commands.py @@ -468,3 +468,13 @@ def run_signals(endpoint, filters, signals): for i in api.get_instances(endpoint, **filters): api._run_signals(endpoint, i, signals, raise_error=True, create_record=False) + + +@click.command(hidden=True) +@options.ANALYSIS_PRIMARY_KEY +@click.option("--reason", help="Rejection reason. (Will be stored in Analysis.notes)") +def reject_analysis(key, reason): + """Patch an analysis status as REJECTED, providing the rejection reason.""" + analysis = api.get_instance("analyses", key) + api.patch_analysis_status(analysis, "REJECTED") + api.patch_instance("analyses", key, {"notes": reason}) diff --git a/isabl_cli/options.py b/isabl_cli/options.py index 513a2cb..c58651b 100644 --- a/isabl_cli/options.py +++ b/isabl_cli/options.py @@ -49,6 +49,7 @@ def _get_experiments(filter_tuples): "STARTED", "SUBMITTED", "SUCCEEDED", + "REJECTED", ] ), ) diff --git a/tests/test_commands.py b/tests/test_commands.py index 4476610..dc60824 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -180,7 +180,7 @@ def test_get_bams(): assert "/hello/mars" in result.output -def test_get_data(tmpdir): +def test_get_data(): runner = CliRunner() experiment = api.create_instance("experiments", **factories.ExperimentFactory()) experiment = data.update_storage_url("experiments", experiment.pk) @@ -287,7 +287,7 @@ def test_run_web_signals(): def test_process_finished_tags(tmpdir): - # check that a tagged analysis does NOT get processed to FINISHED + """Check that a tagged analysis does NOT get processed to FINISHED.""" analysis = api.create_instance( "analyses", project_level_analysis=factories.ProjectFactory(), @@ -310,7 +310,7 @@ def test_process_finished_tags(tmpdir): def test_force_process_finished_tags(tmpdir): - # check that a tagged analysis does get processed to FINISHED when forced + """Check that a tagged analysis does get processed to FINISHED when forced.""" analysis = api.create_instance( "analyses", project_level_analysis=factories.ProjectFactory(), @@ -324,3 +324,19 @@ def test_force_process_finished_tags(tmpdir): print(result.output) analysis = api.get_instance("analyses", analysis["pk"]) assert analysis["status"] == "SUCCEEDED" + + +def test_rejected_analysis(tmpdir): + """Check an analysis can be rejected using the command cli.""" + analysis = api.create_instance( + "analyses", + status="FINISHED", + **factories.AnalysisFactory(ran_by=None), + ) + runner = CliRunner() + rejection_reason = "This analysis is FAKE." + args = ["--key", analysis["pk"], "--reason", rejection_reason] + runner.invoke(commands.reject_analysis, args, catch_exceptions=False) + analysis = api.get_instance("analyses", analysis["pk"]) + assert analysis["status"] == "REJECTED" + assert analysis["notes"] == rejection_reason From 51f24bdd876f389d4fe8748fb5e3ca1060248b65 Mon Sep 17 00:00:00 2001 From: "Juan E. Arango Ossa" Date: Tue, 24 Oct 2023 18:16:55 -0400 Subject: [PATCH 2/3] =?UTF-8?q?=E2=9C=85=20add=20sharing=20to=20project=20?= =?UTF-8?q?factories=20for=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- isabl_cli/factories.py | 1 + 1 file changed, 1 insertion(+) diff --git a/isabl_cli/factories.py b/isabl_cli/factories.py index 67936ed..624ef16 100644 --- a/isabl_cli/factories.py +++ b/isabl_cli/factories.py @@ -22,6 +22,7 @@ class ProjectFactory(BaseFactory): owner = factory.Sequence(lambda n: f"owner-{n}@test.com") principal_investigator = factory.Sequence(lambda n: f"pi-{n}@test.com") title = fuzzy.FuzzyText(length=20, chars=string.ascii_lowercase + " ") + sharing = {"can_share": [], "can_read": [], "is_public": True} class CenterFactory(BaseFactory): From c59cc91325aa96d132bfe3deb4f1129f02522a18 Mon Sep 17 00:00:00 2001 From: "Juan E. Arango Ossa" Date: Tue, 24 Oct 2023 19:17:53 -0400 Subject: [PATCH 3/3] =?UTF-8?q?=E2=9C=85=20fix=20rejected=5Fanalysis=20tes?= =?UTF-8?q?t?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- isabl_cli/api.py | 2 +- isabl_cli/commands.py | 2 +- isabl_cli/settings.py | 5 +++-- tests/test_commands.py | 3 +-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/isabl_cli/api.py b/isabl_cli/api.py index 0940f99..907d84b 100644 --- a/isabl_cli/api.py +++ b/isabl_cli/api.py @@ -628,7 +628,7 @@ def patch_analysis_status(analysis, status): analysis["status"] = status # make sure that the analysis status is updated _set_analysis_permissions(analysis) - if status in {"FAILED", "SUCCEEDED", "IN_PROGRESS"}: + if status in {"FAILED", "SUCCEEDED", "IN_PROGRESS", "REJECTED"}: data["storage_usage"] = utils.get_tree_size(storage_url) if status == "STARTED": diff --git a/isabl_cli/commands.py b/isabl_cli/commands.py index 2694140..2b18111 100644 --- a/isabl_cli/commands.py +++ b/isabl_cli/commands.py @@ -477,4 +477,4 @@ def reject_analysis(key, reason): """Patch an analysis status as REJECTED, providing the rejection reason.""" analysis = api.get_instance("analyses", key) api.patch_analysis_status(analysis, "REJECTED") - api.patch_instance("analyses", key, {"notes": reason}) + api.patch_instance("analyses", key, notes=reason) diff --git a/isabl_cli/settings.py b/isabl_cli/settings.py index f52542f..0086fed 100644 --- a/isabl_cli/settings.py +++ b/isabl_cli/settings.py @@ -56,12 +56,13 @@ "isabl_cli.commands.get_reference", "isabl_cli.commands.get_results", "isabl_cli.commands.login", - "isabl_cli.commands.merge_project_analyses", "isabl_cli.commands.merge_individual_analyses", + "isabl_cli.commands.merge_project_analyses", "isabl_cli.commands.patch_status", + "isabl_cli.commands.reject_analysis", "isabl_cli.commands.rerun_signals", - "isabl_cli.commands.run_web_signals", "isabl_cli.commands.run_signals", + "isabl_cli.commands.run_web_signals", ], "EXTRA_RAW_DATA_FORMATS": [], } diff --git a/tests/test_commands.py b/tests/test_commands.py index dc60824..4f2235e 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -1,6 +1,4 @@ from click.testing import CliRunner -import pytest -import click from isabl_cli import api from isabl_cli import commands @@ -331,6 +329,7 @@ def test_rejected_analysis(tmpdir): analysis = api.create_instance( "analyses", status="FINISHED", + storage_url=tmpdir.strpath, **factories.AnalysisFactory(ran_by=None), ) runner = CliRunner()