From ef8f6d5ab7136d794458905e6b9dbfec1257934e Mon Sep 17 00:00:00 2001 From: Johan Dahlberg Date: Tue, 23 Feb 2016 15:04:35 +0100 Subject: [PATCH 1/5] Added bcl2fastq log pickup endpoint --- bcl2fastq/app.py | 3 +- bcl2fastq/handlers/bcl2fastq_handlers.py | 24 ++++++++++++++-- bcl2fastq/lib/bcl2fastq_logs.py | 18 ++++++++++++ tests/test_bcl2fastq_handlers.py | 7 +++++ tests/test_bcl2fastq_logs.py | 35 ++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 bcl2fastq/lib/bcl2fastq_logs.py create mode 100644 tests/test_bcl2fastq_logs.py diff --git a/bcl2fastq/app.py b/bcl2fastq/app.py index 940d470..c8846a3 100644 --- a/bcl2fastq/app.py +++ b/bcl2fastq/app.py @@ -15,7 +15,8 @@ def routes(**kwargs): url(r"/api/1.0/versions", VersionsHandler, name="versions", kwargs=kwargs), url(r"/api/1.0/start/([\w_-]+)", StartHandler, name="start", kwargs=kwargs), url(r"/api/1.0/status/(\d*)", StatusHandler, name="status", kwargs=kwargs), - url(r"/api/1.0/stop/([\d|all]*)", StopHandler, name="stop", kwargs=kwargs) + url(r"/api/1.0/stop/([\d|all]*)", StopHandler, name="stop", kwargs=kwargs), + url(r"/api/1.0/logs/([\w_-]+)", Bcl2FastqLogHandler, name="logs", kwargs=kwargs) ] def start(): diff --git a/bcl2fastq/handlers/bcl2fastq_handlers.py b/bcl2fastq/handlers/bcl2fastq_handlers.py index cca91d9..6d11b57 100644 --- a/bcl2fastq/handlers/bcl2fastq_handlers.py +++ b/bcl2fastq/handlers/bcl2fastq_handlers.py @@ -6,9 +6,11 @@ from bcl2fastq.lib.jobrunner import LocalQAdapter from bcl2fastq.lib.bcl2fastq_utils import BCL2FastqRunnerFactory, Bcl2FastqConfig from bcl2fastq import __version__ as version +from bcl2fastq.lib.bcl2fastq_logs import Bcl2FastqLogFileProvider from arteria.web.state import State from arteria.web.handlers import BaseRestHandler + log = logging.getLogger(__name__) class Bcl2FastqServiceMixin: @@ -59,6 +61,7 @@ def initialize(self, config): to subclasses. """ self.config = config + self.bcl2fastq_log_file_provider = Bcl2FastqLogFileProvider(self.config) class VersionsHandler(BaseBcl2FastqHandler): @@ -172,8 +175,7 @@ def post(self, runfolder): cmd = job_runner.construct_command() job_runner.symlink_output_to_unaligned() - log_base_path = self.config["bcl2fastq_logs_path"] - log_file = "{0}/{1}.log".format(log_base_path, runfolder) + log_file = self.bcl2fastq_log_file_provider.log_file_path(runfolder) job_id = self.runner_service().start( cmd, @@ -258,4 +260,22 @@ def post(self, job_id): self.send_error(500, reason=e.message) +class Bcl2FastqLogHandler(BaseBcl2FastqHandler): + """ + Gets the content of the log for a particular runfolder + """ + def get(self, runfolder): + """ + Get the content of the log for a particular runfolder + :param runfolder: + :return: + """ + try: + log_content = self.bcl2fastq_log_file_provider.get_log_for_runfolder(runfolder) + response_data = {"runfolder": runfolder, "log": log_content} + self.set_status(200) + self.write_json(response_data) + except IOError as e: + log.warning("Problem with accessing {}, message: {}".format(runfolder, e.message)) + self.send_error(500, reason=e.message) diff --git a/bcl2fastq/lib/bcl2fastq_logs.py b/bcl2fastq/lib/bcl2fastq_logs.py new file mode 100644 index 0000000..9be66db --- /dev/null +++ b/bcl2fastq/lib/bcl2fastq_logs.py @@ -0,0 +1,18 @@ + + +class Bcl2FastqLogFileProvider: + + def __init__(self, config): + self.config = config + + def log_file_path(self, runfolder): + log_base_path = self.config["bcl2fastq_logs_path"] + log_file = "{0}/{1}.log".format(log_base_path, runfolder) + return log_file + + def get_log_for_runfolder(self, runfolder): + log_path = self.log_file_path(runfolder) + with open(log_path) as f: + file_content = f.read() + return file_content + diff --git a/tests/test_bcl2fastq_handlers.py b/tests/test_bcl2fastq_handlers.py index 119c3f1..ec41107 100644 --- a/tests/test_bcl2fastq_handlers.py +++ b/tests/test_bcl2fastq_handlers.py @@ -7,6 +7,7 @@ from bcl2fastq.handlers.bcl2fastq_handlers import * from bcl2fastq.lib.bcl2fastq_utils import BCL2Fastq2xRunner, BCL2FastqRunner +from bcl2fastq.lib.bcl2fastq_logs import Bcl2FastqLogFileProvider from bcl2fastq.app import routes from tornado.web import Application from test_utils import FakeRunner @@ -129,3 +130,9 @@ def test_stop_handler(self): def test_exception_stop_handler(self): response = self.fetch(self.API_BASE + "/stop/lll", method="POST", body = "") self.assertEqual(response.code, 500) + + def test_get_logs(self): + with mock.patch.object(Bcl2FastqLogFileProvider, "get_log_for_runfolder", return_value="This is a string"): + response = self.fetch(self.API_BASE + "/logs/coolest_runfolder", method="GET") + self.assertEqual(response.code, 200) + self.assertEqual(json.loads(response.body)["log"], "This is a string") diff --git a/tests/test_bcl2fastq_logs.py b/tests/test_bcl2fastq_logs.py new file mode 100644 index 0000000..521b882 --- /dev/null +++ b/tests/test_bcl2fastq_logs.py @@ -0,0 +1,35 @@ + +import unittest +from mock import MagicMock, patch, mock_open + +from bcl2fastq.lib.bcl2fastq_logs import Bcl2FastqLogFileProvider + +class TestBcl2FastqLogFileProvider(unittest.TestCase): + + fake_file_content = """ + This is a nice multi-line + string for + you + my friend + """ + fake_path = "/fake/path" + mock_config = MagicMock() + mock_config.__getitem__.return_value = fake_path + runfolder = "160218_ST-E00215_0070_BHKGLFCCXX" + + log_filer_provider = Bcl2FastqLogFileProvider(mock_config) + + def test_log_file_path(self): + log_file = self.log_filer_provider.log_file_path(self.runfolder) + self.assertEqual(log_file, "{}/{}.log".format(self.fake_path, self.runfolder)) + + def test_get_log_for_runfolder(self): + with patch("__builtin__.open", mock_open(read_data=self.fake_file_content), create=True): + file_content = self.log_filer_provider.get_log_for_runfolder(self.runfolder) + self.assertEqual(file_content, self.fake_file_content) + + def test_get_log_for_runfolder_does_not_exist(self): + with self.assertRaises(IOError): + self.log_filer_provider.get_log_for_runfolder(self.runfolder) + + From a676b132efc67ebc683dd0faf158a6eaedb3aa4c Mon Sep 17 00:00:00 2001 From: Johan Dahlberg Date: Tue, 23 Feb 2016 15:13:49 +0100 Subject: [PATCH 2/5] Testing other resources not reachable --- tests/test_bcl2fastq_handlers.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_bcl2fastq_handlers.py b/tests/test_bcl2fastq_handlers.py index ec41107..a1367e8 100644 --- a/tests/test_bcl2fastq_handlers.py +++ b/tests/test_bcl2fastq_handlers.py @@ -136,3 +136,7 @@ def test_get_logs(self): response = self.fetch(self.API_BASE + "/logs/coolest_runfolder", method="GET") self.assertEqual(response.code, 200) self.assertEqual(json.loads(response.body)["log"], "This is a string") + + def test_get_logs_trying_to_reach_other_files(self): + response = self.fetch(self.API_BASE + "/logs/../../../etc/shadow", method="GET") + self.assertEqual(response.code, 404) From ed8800d18b0b9469b35a15fe8cc1aa63be066fd0 Mon Sep 17 00:00:00 2001 From: Johan Dahlberg Date: Tue, 23 Feb 2016 12:56:09 +0100 Subject: [PATCH 3/5] Implement using ArteriaUsageException --- bcl2fastq/handlers/bcl2fastq_handlers.py | 10 ++++++---- bcl2fastq/lib/bcl2fastq_utils.py | 10 +++++++--- requirements/prod | 2 +- tests/tests_bcl2fastq_utils.py | 2 +- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/bcl2fastq/handlers/bcl2fastq_handlers.py b/bcl2fastq/handlers/bcl2fastq_handlers.py index 6d11b57..7a671c9 100644 --- a/bcl2fastq/handlers/bcl2fastq_handlers.py +++ b/bcl2fastq/handlers/bcl2fastq_handlers.py @@ -7,6 +7,7 @@ from bcl2fastq.lib.bcl2fastq_utils import BCL2FastqRunnerFactory, Bcl2FastqConfig from bcl2fastq import __version__ as version from bcl2fastq.lib.bcl2fastq_logs import Bcl2FastqLogFileProvider +from arteria.exceptions import ArteriaUsageException from arteria.web.state import State from arteria.web.handlers import BaseRestHandler @@ -112,7 +113,7 @@ def create_config_from_request(self, runfolder, request_body): runfolder_input = "{0}/{1}".format(runfolder_base_path, runfolder) if not os.path.isdir(runfolder_input): - raise RuntimeError("No such file: {0}".format(runfolder_input)) + raise ArteriaException("No such file: {0}".format(runfolder_input)) if "bcl2fastq_version" in request_data: bcl2fastq_version = request_data["bcl2fastq_version"] @@ -204,11 +205,12 @@ def post(self, runfolder): self.set_status(202, reason="started processing") self.write_json(response_data) - except RuntimeError as e: + except ArteriaException as e: log.warning("Failed starting {0}. Message: {1}".format(runfolder, e.message)) self.send_error(status_code=500, reason=e.message) + class StatusHandler(BaseBcl2FastqHandler, Bcl2FastqServiceMixin): """ Get the status of one or all jobs. @@ -254,8 +256,8 @@ def post(self, job_id): self.runner_service().stop(job_id) self.set_status(200) else: - ValueError("Unknown job to stop") - except ValueError as e: + ArteriaException("Unknown job to stop") + except ArteriaException as e: log.warning("Failed stopping job: {}. Message: ".format(job_id, e.message)) self.send_error(500, reason=e.message) diff --git a/bcl2fastq/lib/bcl2fastq_utils.py b/bcl2fastq/lib/bcl2fastq_utils.py index 70e3546..e54c14f 100644 --- a/bcl2fastq/lib/bcl2fastq_utils.py +++ b/bcl2fastq/lib/bcl2fastq_utils.py @@ -7,6 +7,8 @@ from illuminate.metadata import InteropMetadata + +from arteria.exceptions import ArteriaUsageException from bcl2fastq.lib.illumina import Samplesheet log = logging.getLogger(__name__) @@ -169,7 +171,8 @@ def build_index_string(length_tuple): difference = length_of_index_read - length_of_index_in_samplesheet if not difference >= 0: - raise ValueError("Sample sheet indicates that index is longer than what was read by the sequencer!") + raise ArteriaUsageException("Sample sheet indicates that index is " + "longer than what was read by the sequencer!") if length_of_index_in_samplesheet == 0: # If there is no index in the samplesheet, ignore it in the base-mask @@ -427,8 +430,9 @@ def construct_command(self): is_single_read_run) base_masks_as_set = set(lanes_and_base_mask.values()) - assert len(base_masks_as_set) is 1, "For bcl2fastq 1.8.4 there is no support for " \ - "mixing different bases masks for different lanes" + if len(base_masks_as_set) is 1: + raise ArteriaException("For bcl2fastq 1.8.4 there is no support for " + "mixing different bases masks for different lanes") # Here we are forced to use the same bases mask was always used for all lanes. commandline_collection.append("--use_bases_mask " + lanes_and_base_mask.values()[0]) diff --git a/requirements/prod b/requirements/prod index 17488ae..3da3c7a 100644 --- a/requirements/prod +++ b/requirements/prod @@ -1,7 +1,7 @@ jsonpickle==0.9.2 tornado==4.2.1 git+https://github.com/johandahlberg/localq.git@with_shell_true # Get from pip in future - localq -git+https://github.com/arteria-project/arteria-core.git@v1.0.0#egg=arteria-core +git+https://github.com/arteria-project/arteria-core.git@v1.0.1#egg=arteria-core illuminate==0.6.2 pandas==0.14.1 diff --git a/tests/tests_bcl2fastq_utils.py b/tests/tests_bcl2fastq_utils.py index 566724b..aed3197 100644 --- a/tests/tests_bcl2fastq_utils.py +++ b/tests/tests_bcl2fastq_utils.py @@ -93,7 +93,7 @@ def test_get_bases_mask_per_lane_from_samplesheet_invalid_length_combo(self): mock_read_index_lengths = {2: 4, 3: 4} samplesheet = Samplesheet(TestBcl2FastqConfig.samplesheet_file) - with self.assertRaises(ValueError): + with self.assertRaises(ArteriaUsageException): Bcl2FastqConfig. \ get_bases_mask_per_lane_from_samplesheet(samplesheet, mock_read_index_lengths, False) From 8d8512a4ec1f3dd11249ca6efd9ba0f9a9108a7f Mon Sep 17 00:00:00 2001 From: Johan Dahlberg Date: Tue, 23 Feb 2016 16:31:08 +0100 Subject: [PATCH 4/5] Fixed missed exception renames --- bcl2fastq/handlers/bcl2fastq_handlers.py | 8 ++++---- bcl2fastq/lib/bcl2fastq_utils.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bcl2fastq/handlers/bcl2fastq_handlers.py b/bcl2fastq/handlers/bcl2fastq_handlers.py index 7a671c9..574eddc 100644 --- a/bcl2fastq/handlers/bcl2fastq_handlers.py +++ b/bcl2fastq/handlers/bcl2fastq_handlers.py @@ -113,7 +113,7 @@ def create_config_from_request(self, runfolder, request_body): runfolder_input = "{0}/{1}".format(runfolder_base_path, runfolder) if not os.path.isdir(runfolder_input): - raise ArteriaException("No such file: {0}".format(runfolder_input)) + raise ArteriaUsageException("No such file: {0}".format(runfolder_input)) if "bcl2fastq_version" in request_data: bcl2fastq_version = request_data["bcl2fastq_version"] @@ -205,7 +205,7 @@ def post(self, runfolder): self.set_status(202, reason="started processing") self.write_json(response_data) - except ArteriaException as e: + except ArteriaUsageException as e: log.warning("Failed starting {0}. Message: {1}".format(runfolder, e.message)) self.send_error(status_code=500, reason=e.message) @@ -256,8 +256,8 @@ def post(self, job_id): self.runner_service().stop(job_id) self.set_status(200) else: - ArteriaException("Unknown job to stop") - except ArteriaException as e: + ArteriaUsageException("Unknown job to stop") + except ArteriaUsageException as e: log.warning("Failed stopping job: {}. Message: ".format(job_id, e.message)) self.send_error(500, reason=e.message) diff --git a/bcl2fastq/lib/bcl2fastq_utils.py b/bcl2fastq/lib/bcl2fastq_utils.py index e54c14f..1d74fc4 100644 --- a/bcl2fastq/lib/bcl2fastq_utils.py +++ b/bcl2fastq/lib/bcl2fastq_utils.py @@ -431,7 +431,7 @@ def construct_command(self): base_masks_as_set = set(lanes_and_base_mask.values()) if len(base_masks_as_set) is 1: - raise ArteriaException("For bcl2fastq 1.8.4 there is no support for " + raise ArteriaUsageException("For bcl2fastq 1.8.4 there is no support for " "mixing different bases masks for different lanes") # Here we are forced to use the same bases mask was always used for all lanes. From b0673981b959fc18b2ef6a0514322cde04e1b9f3 Mon Sep 17 00:00:00 2001 From: Johan Hermansson Date: Tue, 8 Mar 2016 14:43:41 +0100 Subject: [PATCH 5/5] Bump version for new release --- bcl2fastq/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bcl2fastq/__init__.py b/bcl2fastq/__init__.py index 6849410..a82b376 100644 --- a/bcl2fastq/__init__.py +++ b/bcl2fastq/__init__.py @@ -1 +1 @@ -__version__ = "1.1.0" +__version__ = "1.1.1"