DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v1] dts: clean up close in remote session
@ 2024-04-23 12:20 Juraj Linkeš
  2024-04-30 17:17 ` Jeremy Spewock
  2024-05-02 10:05 ` Luca Vizzarro
  0 siblings, 2 replies; 3+ messages in thread
From: Juraj Linkeš @ 2024-04-23 12:20 UTC (permalink / raw)
  To: thomas, Honnappa.Nagarahalli, jspewock, probb, paul.szczepanek,
	Luca.Vizzarro, npratte
  Cc: dev, Juraj Linkeš

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>
---
 .../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:
-- 
2.34.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] dts: clean up close in remote session
  2024-04-23 12:20 [PATCH v1] dts: clean up close in remote session Juraj Linkeš
@ 2024-04-30 17:17 ` Jeremy Spewock
  2024-05-02 10:05 ` Luca Vizzarro
  1 sibling, 0 replies; 3+ messages in thread
From: Jeremy Spewock @ 2024-04-30 17:17 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Honnappa.Nagarahalli, probb, paul.szczepanek,
	Luca.Vizzarro, npratte, dev

Makes perfect sense to me to just get rid of the unneeded method and
argument, and the new order also makes things more clear.

Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] dts: clean up close in remote session
  2024-04-23 12:20 [PATCH v1] dts: clean up close in remote session Juraj Linkeš
  2024-04-30 17:17 ` Jeremy Spewock
@ 2024-05-02 10:05 ` Luca Vizzarro
  1 sibling, 0 replies; 3+ messages in thread
From: Luca Vizzarro @ 2024-05-02 10:05 UTC (permalink / raw)
  To: Juraj Linkeš,
	thomas, Honnappa.Nagarahalli, jspewock, probb, paul.szczepanek,
	npratte
  Cc: dev

Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-05-02 10:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-23 12:20 [PATCH v1] dts: clean up close in remote session Juraj Linkeš
2024-04-30 17:17 ` Jeremy Spewock
2024-05-02 10:05 ` Luca Vizzarro

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).