From patchwork Tue Apr 23 12:20:04 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: 139643 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 A592443E82; Tue, 23 Apr 2024 14:20:09 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 63EB542E71; Tue, 23 Apr 2024 14:20:09 +0200 (CEST) Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) by mails.dpdk.org (Postfix) with ESMTP id 35F78402AB for ; Tue, 23 Apr 2024 14:20:08 +0200 (CEST) Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-a5557e3ebcaso916364366b.1 for ; Tue, 23 Apr 2024 05:20:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1713874808; x=1714479608; darn=dpdk.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=+8Jlq4qrOC9z3+UDSiyHJ+YibaKADKMniHu1p/ZgiqU=; b=c8MH/Lnz5oeOhpC6GIlQsSB/cEDIiGEWkqP8+qJltDHO/z+l+0KZE4j6+C9w1IcMks 1KODWL8mnosrPvvKLMLHPZvOTsI32cxA5jrhkhGwQV4x9tppAFuVYeYBaYjzaWcjyjZ7 XJ3Lrhi4TvVu2f9aS/3UrKfI1zYsfz4jze/335nF19s0V1tdjhWAGp0d66iWAy5x/F7H V1SZ2haja6l6byOWPvw6huqXoLn/f616TynrAMwqpasWm9oayp9q+aa/gIEHQQGK6hi1 GWO2+2pO+3o4NXgSPSLMfviNVyJJqMP2+slpXAr91kmnUaQCt9EXViJ07K1QvtUOSWcb vm+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713874808; x=1714479608; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=+8Jlq4qrOC9z3+UDSiyHJ+YibaKADKMniHu1p/ZgiqU=; b=jH7DfTU5kAm3kbjPvTqj+OaocBB0cscKjdj7rCiKJnsAOiib7jg1i2mnsh2glA9RTJ 57d/mnMk/Vk7vaxxHmEfZ9WCMEGa77Cd5NO1sb48bcKPioZ6cs8eN0PR8gK46FbBzsHL 2w9GmJS4kpRi+09131YtoR64ep08fKjbb9aLaiWNRQ7R62jjMUb572yslJwewtgwAXDH 9uaXP5glghVawTerUxmI6iXaXCOElyaywG4JDfOo9jy1eRsVu1I+6WjvbU4Z0r0hPWrk ISvEEubAYwCzAOOPSAQzqkXOYAtfwUhQkRz6Zr+D8z3LQSUgh7x/gZ0PF9bdD+rsY+TZ YbyQ== X-Gm-Message-State: AOJu0YyprTtAzSiBYT/DTaZKZsshNExGN7qWGtgUPl8EZy+oE0W8otGW stYO4h30C/RpX8QmP73G575kVSpKskVBYPc2nruIWE1t+RvLNunpAn4Sl+AuqPQ= X-Google-Smtp-Source: AGHT+IHs1YifFZ6ez55Cmf1WLzLq9/wW0DkOhBBQsfFwb2W64p+tVzrB2dxA8yKuEF+QF4t7lLqNvQ== X-Received: by 2002:a17:906:52c2:b0:a55:b67c:bd04 with SMTP id w2-20020a17090652c200b00a55b67cbd04mr2653744ejn.4.1713874807777; Tue, 23 Apr 2024 05:20:07 -0700 (PDT) Received: from jlinkes-PT-Latitude-5530.pantheon.local (81.89.53.154.host.vnet.sk. [81.89.53.154]) by smtp.gmail.com with ESMTPSA id r12-20020a170906280c00b00a55a57712b0sm3575112ejc.178.2024.04.23.05.20.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Apr 2024 05:20:07 -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?= Subject: [PATCH v1] dts: clean up close in remote session Date: Tue, 23 Apr 2024 14:20:04 +0200 Message-Id: <20240423122004.75240-1-juraj.linkes@pantheon.tech> X-Mailer: git-send-email 2.34.1 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 ad0f53720a..8171f67849 100644 --- a/dts/framework/remote_session/remote_session.py +++ b/dts/framework/remote_session/remote_session.py @@ -190,23 +190,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.""" @@ -242,3 +225,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 782220092c..107740790a 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 d5bf7e0401..7510760de6 100644 --- a/dts/framework/testbed_model/os_session.py +++ b/dts/framework/testbed_model/os_session.py @@ -86,14 +86,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() @@ -159,6 +151,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: