From e20226f331b580fe684ae58001073458a6f06062 Mon Sep 17 00:00:00 2001 From: cliffmccarthy <16453869+cliffmccarthy@users.noreply.github.com> Date: Wed, 12 Nov 2025 11:42:04 -0600 Subject: [PATCH] refactor: Simplify interface to Deployer.install() - In the current code, the only class using the interface that sets need_restart() from the return value of the install() method was IrohDeployer. That interface was created when the install method was a static method, but now it is an instance method with access to 'self'. Therefore, we don't need to pass anything up to the caller to have them set the attribute, we can just set it. - Revised IrohDeployer.install() to set self.need_restart directly, rather than returning a value. - Revised Deployment.install() to ignore the return value of the deployers' install() methods. - need_restart is still present in the base Deployer class to ensure that it is always defined, even when classes do not set it in a constructor. Apart from this initialization for convenience, there is no longer any specific exposure of need_restart in the interface of the Deployer class. - In general, install() methods should use 'self' as little as possible, preferably not at all. In particular, install() methods should never depend on "config" data, such as the config dictionary in self.config or specific values like self.mail_domain. This ensures that these methods can be used to perform generic installation operations that are applicable across multiple relay deployments, and therefore can be called in the process of building a general-purpose container image. --- cmdeploy/src/cmdeploy/__init__.py | 6 +----- cmdeploy/src/cmdeploy/deployer.py | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/cmdeploy/src/cmdeploy/__init__.py b/cmdeploy/src/cmdeploy/__init__.py index 7e8fe14c..fe88115c 100644 --- a/cmdeploy/src/cmdeploy/__init__.py +++ b/cmdeploy/src/cmdeploy/__init__.py @@ -864,11 +864,7 @@ class IrohDeployer(Deployer): ], ) - # - # This will set need_restart when called from an object's - # install() method. - # - return True + self.need_restart = True def configure(self): systemd_unit = files.put( diff --git a/cmdeploy/src/cmdeploy/deployer.py b/cmdeploy/src/cmdeploy/deployer.py index 70d66efe..a55eb4ef 100644 --- a/cmdeploy/src/cmdeploy/deployer.py +++ b/cmdeploy/src/cmdeploy/deployer.py @@ -27,9 +27,7 @@ class Deployment: system=True, ) - ret = bool(deployer.install()) - if ret: - deployer.need_restart = True + deployer.install() def configure(self, deployer): deployer.configure()