mbox series

[v6,0/3] Improve interactive shell output gathering and logging

Message ID 20240724183914.436188-1-jspewock@iol.unh.edu (mailing list archive)
Headers
Series Improve interactive shell output gathering and logging |

Message

Jeremy Spewock July 24, 2024, 6:39 p.m. UTC
From: Jeremy Spewock <jspewock@iol.unh.edu>

v6:
 * Fix error catch for retries. This series changed the error that
   is thrown in the case of a timeout, but it was originally overlooked
   that the context manager patch added a catch that is looking for the
   old timeout error. This version fixes the patch by adjusting the
   error that is expected in the context manager patch to match what
   this series changes it to.

Jeremy Spewock (3):
  dts: Improve output gathering in interactive shells
  dts: Add missing docstring from XML-RPC server
  dts: Improve logging for interactive shells

 dts/framework/exception.py                    | 66 ++++++++++++-------
 dts/framework/remote_session/dpdk_shell.py    |  3 +-
 .../single_active_interactive_shell.py        | 60 ++++++++++++-----
 dts/framework/remote_session/testpmd_shell.py |  2 +
 .../testbed_model/traffic_generator/scapy.py  | 50 +++++++++++++-
 5 files changed, 139 insertions(+), 42 deletions(-)
  

Comments

Juraj Linkeš July 26, 2024, 11:01 a.m. UTC | #1
For series:
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

On 24. 7. 2024 20:39, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> v6:
>   * Fix error catch for retries. This series changed the error that
>     is thrown in the case of a timeout, but it was originally overlooked
>     that the context manager patch added a catch that is looking for the
>     old timeout error. This version fixes the patch by adjusting the
>     error that is expected in the context manager patch to match what
>     this series changes it to.
> 

Here's the diff for anyone interested:
diff --git 
a/dts/framework/remote_session/single_active_interactive_shell.py 
b/dts/framework/remote_session/single_active_interactive_shell.py
index 7014444d0c..77a4dcefdf 100644
--- a/dts/framework/remote_session/single_active_interactive_shell.py
+++ b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -150,7 +150,7 @@ def _start_application(self) -> None:
              try:
                  self.send_command(start_command)
                  break
-            except TimeoutError:
+            except InteractiveSSHTimeoutError:
                  self._logger.info(
                      f"Interactive shell failed to start (attempt 
{attempt+1} out of "
                      f"{self._init_attempts})"

self.send_command raises InteractiveSSHTimeoutError (and not 
TimeoutError) which is why we needed this change.

> Jeremy Spewock (3):
>    dts: Improve output gathering in interactive shells
>    dts: Add missing docstring from XML-RPC server
>    dts: Improve logging for interactive shells
> 
>   dts/framework/exception.py                    | 66 ++++++++++++-------
>   dts/framework/remote_session/dpdk_shell.py    |  3 +-
>   .../single_active_interactive_shell.py        | 60 ++++++++++++-----
>   dts/framework/remote_session/testpmd_shell.py |  2 +
>   .../testbed_model/traffic_generator/scapy.py  | 50 +++++++++++++-
>   5 files changed, 139 insertions(+), 42 deletions(-)
>
  
Thomas Monjalon July 29, 2024, 4:56 p.m. UTC | #2
26/07/2024 13:01, Juraj Linkeš:
> For series:
> Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> 
> On 24. 7. 2024 20:39, jspewock@iol.unh.edu wrote:
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> > 
> > v6:
> >   * Fix error catch for retries. This series changed the error that
> >     is thrown in the case of a timeout, but it was originally overlooked
> >     that the context manager patch added a catch that is looking for the
> >     old timeout error. This version fixes the patch by adjusting the
> >     error that is expected in the context manager patch to match what
> >     this series changes it to.
> > 
> 
> Here's the diff for anyone interested:
> diff --git 
> a/dts/framework/remote_session/single_active_interactive_shell.py 
> b/dts/framework/remote_session/single_active_interactive_shell.py
> index 7014444d0c..77a4dcefdf 100644
> --- a/dts/framework/remote_session/single_active_interactive_shell.py
> +++ b/dts/framework/remote_session/single_active_interactive_shell.py
> @@ -150,7 +150,7 @@ def _start_application(self) -> None:
>               try:
>                   self.send_command(start_command)
>                   break
> -            except TimeoutError:
> +            except InteractiveSSHTimeoutError:
>                   self._logger.info(
>                       f"Interactive shell failed to start (attempt 
> {attempt+1} out of "
>                       f"{self._init_attempts})"
> 
> self.send_command raises InteractiveSSHTimeoutError (and not 
> TimeoutError) which is why we needed this change.
> 
> > Jeremy Spewock (3):
> >    dts: Improve output gathering in interactive shells
> >    dts: Add missing docstring from XML-RPC server
> >    dts: Improve logging for interactive shells

Applied, thanks.