Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cg/exc.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,3 +363,7 @@ class MultipleCaptureKitsError(CgError):

class ApplicationDoesNotHaveHiFiYieldError(CgError):
"""Exception raised when application does not have HiFi yield set."""


class WorkflowVersionCommandFailedError(CgError):
"""Exception raised when a workflow version command fails and the workflow version cannot be retrieved."""
12 changes: 10 additions & 2 deletions cg/services/analysis_starter/submitters/subprocess/submitter.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import logging
import subprocess

from cg.constants import EXIT_SUCCESS
from cg.exc import WorkflowVersionCommandFailedError
from cg.services.analysis_starter.configurator.models.microsalt import MicrosaltCaseConfig
from cg.services.analysis_starter.configurator.models.mip_dna import MIPDNACaseConfig
from cg.services.analysis_starter.submitters.submitter import Submitter
Expand Down Expand Up @@ -43,8 +45,14 @@ def get_workflow_version(case_config: SubprocessCaseConfig) -> str:
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)

if result.returncode != EXIT_SUCCESS:
stderr: str = result.stderr.decode("utf-8").rstrip()
raise WorkflowVersionCommandFailedError(f"Exit code {result.returncode}: {stderr}")

stdout: str = result.stdout.decode("utf-8").rstrip()
return stdout.split()[-1]
except Exception:
LOG.warning(f"Could not retrieve {case_config.workflow} workflow version!")

except Exception as e:
LOG.warning(f"Could not retrieve {case_config.workflow} workflow version: {e}")
return "0.0.0"
133 changes: 130 additions & 3 deletions tests/services/analysis_starter/test_subprocess_submitter.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
import pytest
from pytest_mock import MockerFixture

from cg.constants import Workflow
from cg.constants import EXIT_FAIL, EXIT_SUCCESS, Workflow
from cg.constants.priority import SlurmQos
from cg.exc import WorkflowVersionCommandFailedError
from cg.services.analysis_starter.configurator.models.balsamic import BalsamicCaseConfig
from cg.services.analysis_starter.configurator.models.microsalt import MicrosaltCaseConfig
from cg.services.analysis_starter.configurator.models.mip_dna import MIPDNACaseConfig
Expand Down Expand Up @@ -72,7 +73,7 @@
)


def test_microsalt_get_workflow_version(mocker: MockerFixture):
def test_get_workflow_version_returns_version(mocker: MockerFixture):
# GIVEN a SubprocessSubmitter
subprocess_submitter = SubprocessSubmitter()

Expand All @@ -91,7 +92,9 @@
subprocess,
"run",
return_value=create_autospec(
subprocess.CompletedProcess, stdout=b"microSALT, version 4.2.2 \n"
subprocess.CompletedProcess,
stdout=b"microSALT, version 4.2.2 \n",
returncode=EXIT_SUCCESS,
),
)

Expand All @@ -109,3 +112,127 @@

# THEN the workflow version should have been returned
assert workflow_version == "4.2.2"


def test_get_workflow_version_raises_when_command_fails(mocker: MockerFixture):
# GIVEN a SubprocessSubmitter
subprocess_submitter = SubprocessSubmitter()

# GIVEN a microSALT case config
case_config = MicrosaltCaseConfig(
case_id="case_id",
binary="binary",
conda_binary="conda_binary",
config_file="microSALT.yml",
environment="S_microSALT",
fastq_directory="fastq/dir",
)

# GIVEN that running a subprocess does not work
mock_run = mocker.patch.object(
subprocess,
"run",
return_value=create_autospec(
subprocess.CompletedProcess,
stdout=b"Some microSALT error message",
stderr=b"Some error",
returncode=EXIT_FAIL,
),
)

# GIVEN a spy that verifies the error raised
spy = mocker.spy(WorkflowVersionCommandFailedError, "__init__")

# WHEN getting the workflow version
workflow_version = subprocess_submitter.get_workflow_version(case_config)

# THEN WorkflowVersionNotFoundError should have been triggered
spy.assert_called_once_with(mocker.ANY, "Exit code 1: Some error")

# THEN the subprocess should have been called with the expected call
mock_run.assert_called_once_with(
args=f"{case_config.conda_binary} run {case_config.binary} --version",
shell=True,
check=False,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)

Check warning on line 159 in tests/services/analysis_starter/test_subprocess_submitter.py

View check run for this annotation

codefactor.io / CodeFactor

tests/services/analysis_starter/test_subprocess_submitter.py#L153-L159

Function call with shell=True parameter identified, possible security issue. (B604)

# THEN the fallback workflow version should have been returned
assert workflow_version == "0.0.0"


def test_get_workflow_version_returns_fallback_when_stdout_is_empty(mocker: MockerFixture):
# GIVEN a SubprocessSubmitter
subprocess_submitter = SubprocessSubmitter()

# GIVEN a microSALT case config
case_config = MicrosaltCaseConfig(
case_id="case_id",
binary="binary",
conda_binary="conda_binary",
config_file="microSALT.yml",
environment="S_microSALT",
fastq_directory="fastq/dir",
)

# GIVEN that running a subprocess and the stdout is empty
mock_run = mocker.patch.object(
subprocess,
"run",
return_value=create_autospec(
subprocess.CompletedProcess, stdout=b"", stderr=b"Some error", returncode=EXIT_FAIL
),
)

# WHEN getting the workflow version
workflow_version = subprocess_submitter.get_workflow_version(case_config)

# THEN the subprocess should have been called with the expected call
mock_run.assert_called_once_with(
args=f"{case_config.conda_binary} run {case_config.binary} --version",
shell=True,
check=False,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)

Check warning on line 198 in tests/services/analysis_starter/test_subprocess_submitter.py

View check run for this annotation

codefactor.io / CodeFactor

tests/services/analysis_starter/test_subprocess_submitter.py#L192-L198

Function call with shell=True parameter identified, possible security issue. (B604)

# THEN the fallback workflow version should have been returned
assert workflow_version == "0.0.0"


def test_get_workflow_version_returns_fallback_when_binary_not_found(mocker: MockerFixture):
# GIVEN a SubprocessSubmitter
subprocess_submitter = SubprocessSubmitter()

# GIVEN a microSALT case config
case_config = MicrosaltCaseConfig(
case_id="case_id",
binary="binary",
conda_binary="conda_binary",
config_file="microSALT.yml",
environment="S_microSALT",
fastq_directory="fastq/dir",
)

# GIVEN that running a subprocess raises an OSError
mock_run = mocker.patch.object(
subprocess,
"run",
side_effect=OSError("Binary not found"),
)

# WHEN getting the workflow version
workflow_version = subprocess_submitter.get_workflow_version(case_config)

# THEN the subprocess should have been called with the expected call
mock_run.assert_called_once_with(
args=f"{case_config.conda_binary} run {case_config.binary} --version",
shell=True,
check=False,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)

Check warning on line 235 in tests/services/analysis_starter/test_subprocess_submitter.py

View check run for this annotation

codefactor.io / CodeFactor

tests/services/analysis_starter/test_subprocess_submitter.py#L229-L235

Function call with shell=True parameter identified, possible security issue. (B604)

# THEN the fallback workflow version should have been returned
assert workflow_version == "0.0.0"
Loading