DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
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, "Juraj Linkeš" <juraj.linkes@pantheon.tech>,
	"Luca Vizzarro" <luca.vizzarro@arm.com>
Subject: [PATCH v2 5/5] dts: clean up close in remote session
Date: Wed, 19 Jun 2024 15:35:26 +0200	[thread overview]
Message-ID: <20240619133526.28614-6-juraj.linkes@pantheon.tech> (raw)
In-Reply-To: <20240619133526.28614-1-juraj.linkes@pantheon.tech>

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š <juraj.linkes@pantheon.tech>
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
---
 .../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:
-- 
2.34.1


  parent reply	other threads:[~2024-06-19 13:37 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-23  9:12 [PATCH v1 0/4] node and inheritance improvements Juraj Linkeš
2024-04-23  9:12 ` [PATCH v1 1/4] dts: add tg node execution setup and teardown Juraj Linkeš
2024-04-23  9:18   ` Luca Vizzarro
2024-04-30 16:15     ` Jeremy Spewock
2024-04-23  9:12 ` [PATCH v1 2/4] dts: unify class inheritance from object Juraj Linkeš
2024-04-23  9:19   ` Luca Vizzarro
2024-04-23 14:53   ` Patrick Robb
2024-04-30 16:15     ` Jeremy Spewock
2024-04-23  9:12 ` [PATCH v1 3/4] dts: unify super calls Juraj Linkeš
2024-04-23 10:06   ` Luca Vizzarro
2024-04-23 14:57   ` Patrick Robb
2024-04-30 16:15     ` Jeremy Spewock
2024-04-23  9:12 ` [PATCH v1 4/4] dts: refine pre-test setup and teardown steps Juraj Linkeš
2024-04-23  9:19   ` Luca Vizzarro
2024-04-30 16:15     ` Jeremy Spewock
2024-04-23 10:07 ` [PATCH v1 0/4] node and inheritance improvements Luca Vizzarro
2024-06-19 13:35 ` [PATCH v2 0/5] " Juraj Linkeš
2024-06-19 13:35   ` [PATCH v2 1/5] dts: add tg node test run setup and teardown Juraj Linkeš
2024-06-19 13:35   ` [PATCH v2 2/5] dts: unify class inheritance from object Juraj Linkeš
2024-06-19 13:35   ` [PATCH v2 3/5] dts: unify super calls Juraj Linkeš
2024-06-19 13:35   ` [PATCH v2 4/5] dts: refine pre-test setup and teardown steps Juraj Linkeš
2024-06-19 13:35   ` Juraj Linkeš [this message]
2024-06-20  2:43   ` [PATCH v2 0/5] node and inheritance improvements Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240619133526.28614-6-juraj.linkes@pantheon.tech \
    --to=juraj.linkes@pantheon.tech \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=Luca.Vizzarro@arm.com \
    --cc=dev@dpdk.org \
    --cc=jspewock@iol.unh.edu \
    --cc=npratte@iol.unh.edu \
    --cc=paul.szczepanek@arm.com \
    --cc=probb@iol.unh.edu \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).