From 94a2a2d29cfc0bf8195c4673eba27d4345d7f60e Mon Sep 17 00:00:00 2001 From: Julien LE CLEACH Date: Thu, 4 Apr 2024 14:52:32 +0200 Subject: [PATCH] re-apply extra_args when restarting an application --- CHANGES.md | 2 ++ supvisors/commander.py | 41 +++++++++++++++++++------------ supvisors/internal_com/mapper.py | 7 ++++-- supvisors/tests/test_commander.py | 5 ++-- 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index dbdda949..f5c50a8b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -23,6 +23,8 @@ * Move the host statistics collector to the statistics collector process. The option `stats_collecting_period` is now applicable to host statistics collector. +* Re-apply the eventual process `extra_args` when restarting the application. + * Rework **Supvisors** `RPCInterface` exceptions. diff --git a/supvisors/commander.py b/supvisors/commander.py index cf019687..a1823aa6 100644 --- a/supvisors/commander.py +++ b/supvisors/commander.py @@ -35,7 +35,6 @@ class ProcessCommand: Attributes are: - process: the process wrapped ; - - logger: a reference to the Supvisors logger ; - identifier: the identifier of Supvisors instance where the request has been sent ; - instance_status: the status of the corresponding Supvisors instance ; - request_sequence_counter: the sequence_counter of the Supvisors instance when the request has been performed ; @@ -51,8 +50,7 @@ def __init__(self, process: ProcessStatus) -> None: :param process: the ProcessStatus of the process to command """ - self.process = process - self.logger: Logger = process.logger + self.process: ProcessStatus = process self.identifier: Optional[str] = None self.instance_status: Optional[SupvisorsInstanceStatus] = None self.request_sequence_counter: int = 0 @@ -65,13 +63,20 @@ def __init__(self, process: ProcessStatus) -> None: len(process.supvisors.context.valid_instances()) // 10) self._wait_ticks: int = self.minimum_ticks + @property + def logger(self) -> Logger: + """ Return the Supvisors logger. """ + return self.process.logger + def __str__(self) -> str: """ Get the process command as string. :return: the printable process command """ - return (f'process={self.process.namespec} state={self.process.state_string()} identifier={self.identifier}' - f' request_sequence_counter={self.request_sequence_counter} wait_ticks={self.wait_ticks}') + return (f'process={self.process.namespec} state={self.process.state_string()}' + f' identifier={self.identifier}' + f' request_sequence_counter={self.request_sequence_counter}' + f' wait_ticks={self.wait_ticks}') def __repr__(self) -> str: """ Get the process command as string. @@ -157,7 +162,12 @@ def __init__(self, process: ProcessStatus, strategy: StartingStrategies) -> None # the following attributes are only for Starter self.strategy: StartingStrategies = strategy self.ignore_wait_exit: bool = False - self.extra_args: str = '' + # NOTE: Getting process extra_args here may seem pointless because either the process is started + # by a start process command that will reset or override the extra_args, or the process is started + # through a start application command where it is not possible to pass extra_args. + # However, there is a use case when the process is alone in the application, started by a process command + # with extra args, and a restart application is called. In this case, it makes sense to re-apply the args. + self.extra_args: str = process.extra_args def __str__(self) -> str: """ Get the process command as string. @@ -1073,7 +1083,7 @@ def start_application(self, strategy: StartingStrategies, application: Applicati :param trigger: a status telling if the jobs have to be triggered directly or not. :return: None """ - self.logger.debug(f'Starter.start_application: application={application.application_name}') + self.logger.debug(f'Starter.start_application: {application.application_name}') # push program list in job list and start work if application.stopped(): self.store_application(application, strategy) @@ -1082,8 +1092,7 @@ def start_application(self, strategy: StartingStrategies, application: Applicati if trigger: self.next() else: - self.logger.warn(f'Starter.start_application: application {application.application_name}' - ' already started') + self.logger.warn(f'Starter.start_application: {application.application_name} already started') def store_application(self, application: ApplicationStatus, strategy: StartingStrategies = None) -> bool: """ Copy the start sequence considering programs that are meant to be started automatically, @@ -1130,11 +1139,11 @@ def start_process(self, strategy: StartingStrategies, process: ProcessStatus, ex trigger: bool = True) -> None: """ Plan and start the necessary job to start the process in parameter, with the strategy requested. - :param strategy: the strategy to be used to start the process - :param process: the process to start - :param extra_args: the arguments to be added to the command line - :param trigger: a status telling if the jobs have to be triggered directly or not - :return: None + :param strategy: the strategy to be used to start the process. + :param process: the process to start. + :param extra_args: the arguments to be added to the command line. + :param trigger: a status telling if the jobs have to be triggered directly or not. + :return: None. """ if process.stopped(): self.logger.info(f'Starter.start_process: start {process.namespec} using strategy {strategy.name}') @@ -1262,8 +1271,8 @@ def restart_application(self, strategy: StartingStrategies, application: Applica # trigger stop self.stop_application(application, trigger) else: - self.logger.debug(f'Stopper.restart_application: application={application.application_name} already stopped' - ' so start it directly') + self.logger.debug(f'Stopper.restart_application: application={application.application_name}' + ' already stopped so start it directly') self.supvisors.starter.start_application(strategy, application, trigger) def stop_process(self, process: ProcessStatus, identifiers: NameList = None, trigger: bool = True) -> None: diff --git a/supvisors/internal_com/mapper.py b/supvisors/internal_com/mapper.py index 68b77dd6..84c5253d 100644 --- a/supvisors/internal_com/mapper.py +++ b/supvisors/internal_com/mapper.py @@ -193,9 +193,7 @@ def __init__(self, supvisors: Any): :param supvisors: the global Supvisors structure """ - # keep reference of common logger self.supvisors = supvisors - self.logger: Logger = supvisors.logger # init attributes self._instances: SupvisorsMapper.InstancesMap = OrderedDict() self._nodes: Dict[str, NameList] = {} @@ -204,6 +202,11 @@ def __init__(self, supvisors: Any): self.initial_identifiers: NameList = [] self.stereotypes: Dict[str, NameList] = {} + @property + def logger(self) -> Logger: + """ Return the Supvisors logger. """ + return self.supvisors.logger + @property def local_instance(self) -> SupvisorsInstanceId: """ Property getter for the SupvisorsInstanceId corresponding to local_identifier. diff --git a/supvisors/tests/test_commander.py b/supvisors/tests/test_commander.py index aa993844..2e4796c2 100644 --- a/supvisors/tests/test_commander.py +++ b/supvisors/tests/test_commander.py @@ -111,8 +111,9 @@ def start_command(supvisors): """ Create a ProcessStartCommand instance. """ info = process_info_by_name('xclock') info['startsecs'] = 18 - process = create_process(info, supvisors) + process: ProcessStatus = create_process(info, supvisors) process.add_info('10.0.0.1', info) + process.extra_args = '--help' return ProcessStartCommand(process, StartingStrategies.MOST_LOADED) @@ -126,7 +127,7 @@ def test_start_command_create(start_command): assert start_command._wait_ticks == ProcessCommand.DEFAULT_TICK_TIMEOUT assert start_command.strategy == StartingStrategies.MOST_LOADED assert not start_command.ignore_wait_exit - assert start_command.extra_args == '' + assert start_command.extra_args == '--help' def test_start_command_str(start_command):