diff --git a/pypiper/const.py b/pypiper/const.py index 2749529..0bfbdb6 100644 --- a/pypiper/const.py +++ b/pypiper/const.py @@ -1,6 +1,5 @@ """ Pypiper constants. """ - CHECKPOINT_EXTENSION = ".checkpoint" DEFAULT_SAMPLE_NAME = "DEFAULT_SAMPLE_NAME" PIPELINE_CHECKPOINT_DELIMITER = "_" diff --git a/pypiper/manager.py b/pypiper/manager.py index ce1e385..5aad867 100644 --- a/pypiper/manager.py +++ b/pypiper/manager.py @@ -58,6 +58,7 @@ LOCK_PREFIX = "lock." +LOGFILE_SUFFIX = "_log.md" class Unbuffered(object): @@ -198,10 +199,7 @@ def __init__( # If no starting point was specified, assume that the pipeline's # execution is to begin right away and set the internal flag so that # run() is let loose to execute instructions given. - if not self.start_point: - self._active = True - else: - self._active = False + self._active = not self.start_point # Pipeline-level variables to track global state and pipeline stats # Pipeline settings @@ -215,26 +213,37 @@ def __init__( self.output_parent = params["output_parent"] self.testmode = params["testmode"] + # Establish the log file to check safety with logging keyword arguments. + # Establish the output folder since it's required for the log file. + self.outfolder = os.path.join(outfolder, "") # trailing slash + self.pipeline_log_file = pipeline_filepath(self, suffix=LOGFILE_SUFFIX) + # Set up logger logger_kwargs = logger_kwargs or {} + if logger_kwargs.get("logfile") == self.pipeline_log_file: + raise ValueError( + f"The logfile given for the pipeline manager's logger matches that which will be used by the manager itself: {self.pipeline_log_file}" + ) default_logname = ".".join([__name__, self.__class__.__name__, self.name]) - if not args: + self._logger = None + if args: + logger_builder_method = "logger_via_cli" + try: + self._logger = logger_via_cli(args, **logger_kwargs) + except logmuse.est.AbsentOptionException as e: + # Defer logger construction to init_logger. + self.debug(f"logger_via_cli failed: {e}") + if self._logger is None: + logger_builder_method = "init_logger" + # covers cases of bool(args) being False, or failure of logger_via_cli. # strict is only for logger_via_cli. - kwds = {k: v for k, v in logger_kwargs.items() if k != "strict"} + logger_kwargs = {k: v for k, v in logger_kwargs.items() if k != "strict"} try: - name = kwds.pop("name") + name = logger_kwargs.pop("name") except KeyError: name = default_logname - self._logger = logmuse.init_logger(name, **kwds) - self.debug("Logger set with logmuse.init_logger") - else: - logger_kwargs.setdefault("name", default_logname) - try: - self._logger = logmuse.logger_via_cli(args) - self.debug("Logger set with logmuse.logger_via_cli") - except logmuse.est.AbsentOptionException: - self._logger = logmuse.init_logger("pypiper", level="DEBUG") - self.debug("logger_via_cli failed; Logger set with logmuse.init_logger") + self._logger = logmuse.init_logger(name, **logger_kwargs) + self.debug(f"Logger set with {logger_builder_method}") # Keep track of an ID for the number of processes attempted self.proc_count = 0 @@ -281,10 +290,7 @@ def __init__( # self.output_parent = os.path.join(os.getcwd(), self.output_parent) # File paths: - self.outfolder = os.path.join(outfolder, "") # trailing slash self.make_sure_path_exists(self.outfolder) - self.pipeline_log_file = pipeline_filepath(self, suffix="_log.md") - self.pipeline_profile_file = pipeline_filepath(self, suffix="_profile.tsv") # Stats and figures are general and so lack the pipeline name. diff --git a/pypiper/ngstk.py b/pypiper/ngstk.py index 329b321..b607913 100755 --- a/pypiper/ngstk.py +++ b/pypiper/ngstk.py @@ -153,8 +153,7 @@ def get_file_size(self, filenames): return sum([self.get_file_size(filename) for filename in filenames]) return round( - sum([float(os.stat(f).st_size) for f in filenames.split(" ")]) - / (1024**2), + sum([float(os.stat(f).st_size) for f in filenames.split(" ")]) / (1024**2), 4, ) diff --git a/pypiper/utils.py b/pypiper/utils.py index 8973466..1c65f59 100644 --- a/pypiper/utils.py +++ b/pypiper/utils.py @@ -785,12 +785,10 @@ def pipeline_filepath(pm, filename=None, suffix=None): filename as given or determined by the pipeline name, and suffix appended if given. """ - if filename is None and suffix is None: raise TypeError( - "Provide filename and/or suffix to create " "path to a pipeline file." + "Provide filename and/or suffix to create path to a pipeline file." ) - filename = (filename or pm.name) + (suffix or "") # Note that Pipeline and PipelineManager define the same outfolder. diff --git a/tests/pipeline_manager/test_manager_constructor.py b/tests/pipeline_manager/test_manager_constructor.py index 0792bf1..b327fb2 100644 --- a/tests/pipeline_manager/test_manager_constructor.py +++ b/tests/pipeline_manager/test_manager_constructor.py @@ -1,10 +1,11 @@ """ Test effects of construction of a pipeline manager. """ import argparse +import os import pytest -from pypiper.manager import CHECKPOINT_SPECIFICATIONS +from pypiper.manager import CHECKPOINT_SPECIFICATIONS, LOGFILE_SUFFIX from tests.helpers import named_param __author__ = "Vince Reuter" @@ -24,6 +25,22 @@ def test_manager_starts_in_null_checkpoint_state(get_pipe_manager, checkpoint_ty assert getattr(pm, checkpoint_type) is None +def test_logger_logfile_collision_with_manager_logfile_is_expected_error__issue_212( + get_pipe_manager, tmpdir +): + pipe_name = "test_issue212" + with pytest.raises(ValueError) as err_ctx: + get_pipe_manager( + name=pipe_name, + logger_kwargs={ + "logfile": os.path.join(tmpdir.strpath, pipe_name + LOGFILE_SUFFIX) + }, + ) + assert str(err_ctx.value).startswith( + f"The logfile given for the pipeline manager's logger matches that which will be used by the manager itself" + ) + + class ManagerConstructorCheckpointSpecificationTests: """Tests for manager's constructor's ability to parse and set checkpoint specifications, which can determine aspects of control flow."""