Skip to content

use custom log_file name as per the user setting #861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: rolling
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
15 changes: 11 additions & 4 deletions launch/launch/launch_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ def __init__(
*,
argv: Optional[Iterable[Text]] = None,
noninteractive: bool = False,
debug: bool = False
debug: bool = False,
log_file_name: str = 'launch.log'
) -> None:
"""
Create a LaunchService.
Expand All @@ -67,12 +68,18 @@ def __init__(
"""
# Setup logging and debugging.
launch.logging.launch_config.level = logging.DEBUG if debug else logging.INFO
self._log_file_name = log_file_name
# Ensure the log file name ends with `.log`
if not self._log_file_name.endswith('.log'):
self._log_file_name += '.log'
launch.logging.launch_config.log_file_name = self._log_file_name
# Setup logging
self._logger_name = self._log_file_name.removesuffix('.log')
self.__logger = launch.logging.get_logger(self._logger_name)

self.__debug = debug
self.__argv = argv if argv is not None else []

# Setup logging
self.__logger = launch.logging.get_logger('launch')

# Setup context and register a built-in event handler for bootstrapping.
self.__context = LaunchContext(argv=self.__argv, noninteractive=noninteractive)
self.__context.register_event_handler(OnIncludeLaunchDescription())
Expand Down
23 changes: 20 additions & 3 deletions launch/launch/logging/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ def reset(self):
self.screen_formatter = None
self.file_formatter = None
self._log_handler_factory = None
self._log_file_name = 'launch.log'
logging.root.setLevel(logging.INFO)
self.set_screen_format('default')
self.set_log_format('default')
Expand All @@ -130,6 +131,20 @@ def level(self, new_level):
"""
logging.root.setLevel(new_level)

@property
def log_file_name(self) -> str:
"""Get the current log file name."""
return self._log_file_name

@log_file_name.setter
def log_file_name(self, log_file_name: str):
"""
Set the name of the log file.

:param log_file_name: the name of the log file where logger output should be written.
"""
self._log_file_name = log_file_name

@property
def log_dir(self):
"""Get the current log directory, generating it if necessary."""
Expand Down Expand Up @@ -347,7 +362,7 @@ def get_logger(name=None) -> logging.Logger:
screen_handler = launch_config.get_screen_handler()
if screen_handler not in logger.handlers:
logger.addHandler(screen_handler)
launch_log_file_handler = launch_config.get_log_file_handler()
launch_log_file_handler = launch_config.get_log_file_handler(launch_config.log_file_name)
if launch_log_file_handler not in logger.handlers:
logger.addHandler(launch_log_file_handler)
return logger
Expand Down Expand Up @@ -416,7 +431,7 @@ def _normalize_output_configuration(config):
return normalized_config


def get_output_loggers(process_name, output_config):
def get_output_loggers(process_name, output_config, main_log_file_name='launch.log'):
"""
Get the stdout and stderr output loggers for the given process name.

Expand Down Expand Up @@ -457,6 +472,8 @@ def get_output_loggers(process_name, output_config):
:param process_name: the process-like action whose outputs want to be logged.
:param output_config: configuration for the output loggers,
see above for details.
:param main_log_file_name: the name of the main log file to be used.
Defaults to 'launch.log'.
:returns: a tuple with the stdout and stderr output loggers.
"""
output_config = _normalize_output_configuration(output_config)
Expand All @@ -476,7 +493,7 @@ def get_output_loggers(process_name, output_config):
# If a 'log' output is configured for this source or for
# 'both' sources, this logger should output to launch main log file.
if 'log' in (output_config['both'] | output_config[source]):
launch_log_file_handler = launch_config.get_log_file_handler()
launch_log_file_handler = launch_config.get_log_file_handler(main_log_file_name)
# Add launch main log file handler if necessary.
if launch_log_file_handler not in logger.handlers:
launch_log_file_handler.setFormatterFor(
Expand Down
13 changes: 12 additions & 1 deletion launch/test/launch/test_launch_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,18 @@

def test_launch_service_constructors():
"""Test the constructors for LaunchService class."""
LaunchService()
# When no name is provided for log file
ls = LaunchService()
assert ls._log_file_name == 'launch.log'
assert ls._logger_name == 'launch'
# WHen log file name is provided without .log
ls = LaunchService(log_file_name='custom_logger')
assert ls._log_file_name == 'custom_logger.log'
assert ls._logger_name == 'custom_logger'
# When log file name is provided with .log
ls = LaunchService(log_file_name='custom_logger.log')
assert ls._log_file_name == 'custom_logger.log'
assert ls._logger_name == 'custom_logger'
LaunchService(debug=True)
LaunchService(debug=False)

Expand Down
38 changes: 28 additions & 10 deletions launch/test/launch/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def test_output_loggers_bad_configuration(log_dir):
launch.logging.get_output_loggers('some-proc', {'stdout': {'garbage'}})


@pytest.mark.parametrize('config,checks', [
configs = [
('screen', {'stdout': {'screen'}, 'stderr': {'screen'}}),
('log', {'stdout': {'log'}, 'stderr': {'log', 'screen'}}),
('both', {'both': {'log', 'screen'}}),
Expand All @@ -88,16 +88,33 @@ def test_output_loggers_bad_configuration(log_dir):
'stderr': {'own_log'}
},
)
])
def test_output_loggers_configuration(capsys, log_dir, config, checks, mock_clean_env):
]
log_file_names = ['custom-log-name.log', None]
params = [
(config, checks, log_file_name)
for (config, checks) in configs
for log_file_name in log_file_names
]


@pytest.mark.parametrize('config,checks,main_log_file_name', params)
def test_output_loggers_configuration(
capsys, log_dir, config, checks, main_log_file_name, mock_clean_env
):
checks = {'stdout': set(), 'stderr': set(), 'both': set(), **checks}
launch.logging.reset()
launch.logging.launch_config.log_dir = log_dir
logger = launch.logging.get_logger('some-proc')
if main_log_file_name is None:
main_log_file_name = launch.logging.launch_config.log_file_name
else:
launch.logging.launch_config.log_file_name = main_log_file_name
logger_name = main_log_file_name.removesuffix('.log')
logger = launch.logging.get_logger(logger_name)
logger.addHandler(launch.logging.launch_config.get_screen_handler())
logger.addHandler(launch.logging.launch_config.get_log_file_handler())
logger.addHandler(launch.logging.launch_config.get_log_file_handler(main_log_file_name))
logger.setLevel(logging.ERROR)
stdout_logger, stderr_logger = launch.logging.get_output_loggers('some-proc', config)
stdout_logger, stderr_logger = launch.logging.get_output_loggers(
'some-proc', config, main_log_file_name)

logger.debug('oops')
logger.error('baz')
Expand All @@ -106,21 +123,22 @@ def test_output_loggers_configuration(capsys, log_dir, config, checks, mock_clea

capture = capsys.readouterr()
lines = list(reversed(capture.out.splitlines()))
assert '[ERROR] [some-proc]: baz' == lines.pop()
assert f'[ERROR] [{logger_name}]: baz' == lines.pop()
if 'screen' in (checks['stdout'] | checks['both']):
assert 'foo' == lines.pop()
if 'screen' in (checks['stderr'] | checks['both']):
assert 'bar' == lines.pop()
assert 0 == len(lines)
assert 0 == len(capture.err)

launch.logging.launch_config.get_log_file_handler().flush()
main_log_path = launch.logging.launch_config.get_log_file_path()
launch.logging.launch_config.get_log_file_handler(main_log_file_name).flush()
main_log_path = launch.logging.launch_config.get_log_file_path(main_log_file_name)
assert os.path.exists(main_log_path)
assert 0 != os.stat(main_log_path).st_size
with open(main_log_path, 'r') as f:
lines = list(reversed(f.readlines()))
assert re.match(r'[0-9]+\.[0-9]+ \[ERROR\] \[some-proc\]: baz', lines.pop()) is not None
assert re.match(rf'[0-9]+\.[0-9]+ \[ERROR\] \[{logger_name}\]: baz',
lines.pop()) is not None
if 'log' in (checks['stdout'] | checks['both']):
assert re.match(r'[0-9]+\.[0-9]+ foo', lines.pop()) is not None
if 'log' in (checks['stderr'] | checks['both']):
Expand Down