From patchwork Wed Jun 19 13:35:26 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Juraj_Linke=C5=A1?= X-Patchwork-Id: 141393 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B1E7B454A8; Wed, 19 Jun 2024 15:37:36 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8477942D7F; Wed, 19 Jun 2024 15:37:15 +0200 (CEST) Received: from mail-ed1-f51.google.com (mail-ed1-f51.google.com [209.85.208.51]) by mails.dpdk.org (Postfix) with ESMTP id 872DC4021D for ; Wed, 19 Jun 2024 15:35:35 +0200 (CEST) Received: by mail-ed1-f51.google.com with SMTP id 4fb4d7f45d1cf-57d07f07a27so1044773a12.3 for ; Wed, 19 Jun 2024 06:35:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1718804135; x=1719408935; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=dos7DmZuljpljeDAdZujU/FDqFNJGG6cXh8AVtg97PY=; b=vSMVwvgwn4+4moUlp8S1VS4BmHfySAzpmsCgtVA7NsyE+LTpopAnx4lPpWzZmy927q 5sVqJkgJDYc7c8G0rqxfPrtD3KSQzvb1i+WdkEfh4DE2pKDqeOlgYK+RiRumM/WnmXtW QCipDyl8PRF54R45HpwfXQTIpEvCHDCAa3C+gOyEVjDIKAPZbnkdvCX/gMbF+aYIg9VK ryl/WmQ6/iwd0Au4JC9z5Bxh16DP4j0BFqmsIojVcFkkue8TE4DqF2h/GEmDHaHnbRRA kaiwAaApMlWNcvg8pZcPCGs4NFPLpfoCGZofqPwWPN+C5LCYULNGf/N1qY+BHBozQqYV 6BeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718804135; x=1719408935; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=dos7DmZuljpljeDAdZujU/FDqFNJGG6cXh8AVtg97PY=; b=K7SMT4wTt5WrQPbQcgdrbjwR9A5+MsacmszC89B/XXNHEC8GIQhmZbWKeKMhHCkokU 6XqS85ycPnNiRWDOuC6SbslSybbtowFN4pQ7XuPfxrsP0iefQA536jsVASJdSkdIsLvn HJJ5CupalQWsXqSj1mhsdwUPPAS7feNMlT022IIPTg7lUTglxajph3RPf6kyV/fV3mu5 kWSBSD4B/00/Hv84MqojfI7pjv+9lAme6j6sWfZiRexpXOAsG4LOQOTvoub2k/zUOEEr nZA/hiwqN1SjK6u7EWdJT8LO6bcbFxsjn1tlJxWjlxLaPMmNVaofaeOP/C0sxecLub2q p1Rg== X-Gm-Message-State: AOJu0YzA9yrA/a+uOkQQc4Pgo4E9kzky33BepC4VL1Ybroj97rYEcNut 6dudmdHti2JOin2CzxYJDOO+Ynp+ebAya3rbIcTyH0m55cXeT9VQW07gAsl0EXo= X-Google-Smtp-Source: AGHT+IE/bt8mZov5RiUy8PGnuggiQ4+mMRIAfgAlf/Mog7JemMXNuVsCb+491j7LsP88+7Kr0Hlvrw== X-Received: by 2002:a17:906:f150:b0:a6e:f701:384d with SMTP id a640c23a62f3a-a6fab61cf2amr119582466b.29.1718804135145; Wed, 19 Jun 2024 06:35:35 -0700 (PDT) Received: from localhost.localdomain ([84.245.121.236]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a6f56da3fe5sm676723566b.18.2024.06.19.06.35.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 Jun 2024 06:35:34 -0700 (PDT) From: =?utf-8?q?Juraj_Linke=C5=A1?= To: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com, Luca.Vizzarro@arm.com, npratte@iol.unh.edu Cc: dev@dpdk.org, =?utf-8?q?Juraj_Linke=C5=A1?= , Luca Vizzarro Subject: [PATCH v2 5/5] dts: clean up close in remote session Date: Wed, 19 Jun 2024 15:35:26 +0200 Message-Id: <20240619133526.28614-6-juraj.linkes@pantheon.tech> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240619133526.28614-1-juraj.linkes@pantheon.tech> References: <20240423091252.62924-1-juraj.linkes@pantheon.tech> <20240619133526.28614-1-juraj.linkes@pantheon.tech> MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org The close method was split into two methods, one with common code and one that's subclass specific. There wasn't any common code so the split doesn't make sense. And if we ever need such a split, we can use super() in the future. There was also an unused argument, force and the order of methods was cleaned up a little (close is usually the last thing we call in the lifecycle of a session object, so it was moved to the last place among public methods). Signed-off-by: Juraj Linkeš Reviewed-by: Jeremy Spewock Reviewed-by: Luca Vizzarro --- .../remote_session/remote_session.py | 21 ++++--------------- dts/framework/remote_session/ssh_session.py | 11 +++++----- dts/framework/testbed_model/os_session.py | 12 ++++------- 3 files changed, 14 insertions(+), 30 deletions(-) diff --git a/dts/framework/remote_session/remote_session.py b/dts/framework/remote_session/remote_session.py index 8db26e9efc..8c580b070f 100644 --- a/dts/framework/remote_session/remote_session.py +++ b/dts/framework/remote_session/remote_session.py @@ -191,23 +191,6 @@ def _send_command(self, command: str, timeout: float, env: dict | None) -> Comma * SSHTimeoutError if the command execution times out. """ - def close(self, force: bool = False) -> None: - """Close the remote session and free all used resources. - - Args: - force: Force the closure of the connection. This may not clean up all resources. - """ - self._close(force) - - @abstractmethod - def _close(self, force: bool = False) -> None: - """Protocol specific steps needed to close the session properly. - - Args: - force: Force the closure of the connection. This may not clean up all resources. - This doesn't have to be implemented in the overloaded method. - """ - @abstractmethod def is_alive(self) -> bool: """Check whether the remote session is still responding.""" @@ -243,3 +226,7 @@ def copy_to( source_file: The file on the local filesystem. destination_file: A file or directory path on the remote Node. """ + + @abstractmethod + def close(self) -> None: + """Close the remote session and free all used resources.""" diff --git a/dts/framework/remote_session/ssh_session.py b/dts/framework/remote_session/ssh_session.py index 216bd25aed..66f8176833 100644 --- a/dts/framework/remote_session/ssh_session.py +++ b/dts/framework/remote_session/ssh_session.py @@ -74,10 +74,6 @@ def _connect(self) -> None: else: raise SSHConnectionError(self.hostname, errors) - def is_alive(self) -> bool: - """Overrides :meth:`~.remote_session.RemoteSession.is_alive`.""" - return self.session.is_connected - def _send_command(self, command: str, timeout: float, env: dict | None) -> CommandResult: """Send a command and return the result of the execution. @@ -103,6 +99,10 @@ def _send_command(self, command: str, timeout: float, env: dict | None) -> Comma return CommandResult(self.name, command, output.stdout, output.stderr, output.return_code) + def is_alive(self) -> bool: + """Overrides :meth:`~.remote_session.RemoteSession.is_alive`.""" + return self.session.is_connected + def copy_from( self, source_file: str | PurePath, @@ -119,5 +119,6 @@ def copy_to( """Overrides :meth:`~.remote_session.RemoteSession.copy_to`.""" self.session.put(str(source_file), str(destination_file)) - def _close(self, force: bool = False) -> None: + def close(self) -> None: + """Overrides :meth:`~.remote_session.RemoteSession.close`.""" self.session.close() diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py index bba4aca9ce..34b0a9e749 100644 --- a/dts/framework/testbed_model/os_session.py +++ b/dts/framework/testbed_model/os_session.py @@ -88,14 +88,6 @@ def __init__( self.remote_session = create_remote_session(node_config, name, logger) self.interactive_session = create_interactive_session(node_config, logger) - def close(self, force: bool = False) -> None: - """Close the underlying remote session. - - Args: - force: Force the closure of the connection. - """ - self.remote_session.close(force) - def is_alive(self) -> bool: """Check whether the underlying remote session is still responding.""" return self.remote_session.is_alive() @@ -161,6 +153,10 @@ def create_interactive_shell( timeout, ) + def close(self) -> None: + """Close the underlying remote session.""" + self.remote_session.close() + @staticmethod @abstractmethod def _get_privileged_command(command: str) -> str: