python: add __repr__ to ConsoleSocket to aid debugging

[RFC PATCH] python: add __repr__ to ConsoleSocket to aid debugging

Posted by Alex Bennée 4 years, 11 months ago

While attempting to debug some console weirdness I thought it would be
worth making it easier to see what it had inside.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 python/qemu/console_socket.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index f060d79e06..77966d1fe9 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -45,6 +45,14 @@ class ConsoleSocket(socket.socket):
         if drain:
             self._drain_thread = self._thread_start()
 
+    def __repr__(self):
+        s = super(ConsoleSocket, self).__repr__()
+        s = s.rstrip(">")
+        s += ", logfile=%s" % (self._logfile)
+        s += ", drain_thread=%s" % (self._drain_thread)
+        s += ">"
+        return s
+
     def _drain_fn(self) -> None:
         """Drains the socket and runs while the socket is open."""
         while self._open:
-- 
2.20.1

Re: [RFC PATCH] python: add __repr__ to ConsoleSocket to aid debugging

Posted by John Snow 4 years, 11 months ago

On 12/7/20 3:07 PM, Alex Bennée wrote:
> While attempting to debug some console weirdness I thought it would be
> worth making it easier to see what it had inside.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   python/qemu/console_socket.py | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
> index f060d79e06..77966d1fe9 100644
> --- a/python/qemu/console_socket.py
> +++ b/python/qemu/console_socket.py
> @@ -45,6 +45,14 @@ class ConsoleSocket(socket.socket):
>           if drain:
>               self._drain_thread = self._thread_start()
>   
> +    def __repr__(self):

def __repr__(self) -> str:

> +        s = super(ConsoleSocket, self).__repr__()

Use python3-style super(): super().__repr__()

> +        s = s.rstrip(">")
> +        s += ", logfile=%s" % (self._logfile)
> +        s += ", drain_thread=%s" % (self._drain_thread)
> +        s += ">"
> +        return s
> +

Reviewed-by: John Snow <jsnow@redhat.com>

feel free to take this through your testing tree.

--js

Re: [RFC PATCH] python: add __repr__ to ConsoleSocket to aid debugging

Posted by Willian Rampazzo 4 years, 11 months ago

On Mon, Dec 7, 2020 at 5:10 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> While attempting to debug some console weirdness I thought it would be
> worth making it easier to see what it had inside.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  python/qemu/console_socket.py | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
> index f060d79e06..77966d1fe9 100644
> --- a/python/qemu/console_socket.py
> +++ b/python/qemu/console_socket.py
> @@ -45,6 +45,14 @@ class ConsoleSocket(socket.socket):
>          if drain:
>              self._drain_thread = self._thread_start()
>
> +    def __repr__(self):
> +        s = super(ConsoleSocket, self).__repr__()
> +        s = s.rstrip(">")
> +        s += ", logfile=%s" % (self._logfile)
> +        s += ", drain_thread=%s" % (self._drain_thread)
> +        s += ">"

We could use something more pythonic for this file. Instead of 3
string concatenations, my suggestion is to go with string formatting,
like:

s = "%s,  logfile=%s, drain_thread=%s>" % (s, self._logfile, self._drain_thread)

As str is immutable in Python, it avoids unnecessary copies.

> +        return s
> +
>      def _drain_fn(self) -> None:
>          """Drains the socket and runs while the socket is open."""
>          while self._open:
> --
> 2.20.1
>
>

Re: [RFC PATCH] python: add __repr__ to ConsoleSocket to aid debugging

Posted by John Snow 4 years, 11 months ago

On 12/7/20 4:35 PM, Willian Rampazzo wrote:
> We could use something more pythonic for this file. Instead of 3
> string concatenations, my suggestion is to go with string formatting,
> like:
> 
> s = "%s,  logfile=%s, drain_thread=%s>" % (s, self._logfile, self._drain_thread)
> 
> As str is immutable in Python, it avoids unnecessary copies.

Sure, this is fine too. I'm not too worried about performance of 
debugging methods.

Alex, use your own discretion -- feel free to keep my RB/ACK and merge 
alongside your other tests when ready. Thanks!

--js

Re: [RFC PATCH] python: add __repr__ to ConsoleSocket to aid debugging

Posted by Philippe Mathieu-Daudé 4 years, 11 months ago

Hi Willian,

On 12/7/20 10:35 PM, Willian Rampazzo wrote:
> On Mon, Dec 7, 2020 at 5:10 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> While attempting to debug some console weirdness I thought it would be
>> worth making it easier to see what it had inside.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  python/qemu/console_socket.py | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
>> index f060d79e06..77966d1fe9 100644
>> --- a/python/qemu/console_socket.py
>> +++ b/python/qemu/console_socket.py
>> @@ -45,6 +45,14 @@ class ConsoleSocket(socket.socket):
>>          if drain:
>>              self._drain_thread = self._thread_start()
>>
>> +    def __repr__(self):
>> +        s = super(ConsoleSocket, self).__repr__()
>> +        s = s.rstrip(">")
>> +        s += ", logfile=%s" % (self._logfile)
>> +        s += ", drain_thread=%s" % (self._drain_thread)
>> +        s += ">"
> 
> We could use something more pythonic for this file. Instead of 3
> string concatenations, my suggestion is to go with string formatting,
> like:
> 
> s = "%s,  logfile=%s, drain_thread=%s>" % (s, self._logfile, self._drain_thread)
> 
> As str is immutable in Python, it avoids unnecessary copies.

With this (and John's comment) addressed, are you OK to add
your R-b tag?

> 
>> +        return s
>> +
>>      def _drain_fn(self) -> None:
>>          """Drains the socket and runs while the socket is open."""
>>          while self._open:
>> --
>> 2.20.1
>>
>>
> 
>

Re: [RFC PATCH] python: add __repr__ to ConsoleSocket to aid debugging

Posted by Willian Rampazzo 4 years, 11 months ago

Em seg, 7 de dez de 2020 19:14, Philippe Mathieu-Daudé <philmd@redhat.com>
escreveu:

> Hi Willian,
>
> On 12/7/20 10:35 PM, Willian Rampazzo wrote:
> > On Mon, Dec 7, 2020 at 5:10 PM Alex Bennée <alex.bennee@linaro.org>
> wrote:
> >>
> >> While attempting to debug some console weirdness I thought it would be
> >> worth making it easier to see what it had inside.
> >>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> ---
> >>  python/qemu/console_socket.py | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/python/qemu/console_socket.py
> b/python/qemu/console_socket.py
> >> index f060d79e06..77966d1fe9 100644
> >> --- a/python/qemu/console_socket.py
> >> +++ b/python/qemu/console_socket.py
> >> @@ -45,6 +45,14 @@ class ConsoleSocket(socket.socket):
> >>          if drain:
> >>              self._drain_thread = self._thread_start()
> >>
> >> +    def __repr__(self):
> >> +        s = super(ConsoleSocket, self).__repr__()
> >> +        s = s.rstrip(">")
> >> +        s += ", logfile=%s" % (self._logfile)
> >> +        s += ", drain_thread=%s" % (self._drain_thread)
> >> +        s += ">"
> >
> > We could use something more pythonic for this file. Instead of 3
> > string concatenations, my suggestion is to go with string formatting,
> > like:
> >
> > s = "%s,  logfile=%s, drain_thread=%s>" % (s, self._logfile,
> self._drain_thread)
> >
> > As str is immutable in Python, it avoids unnecessary copies.
>
> With this (and John's comment) addressed, are you OK to add
> your R-b tag?
>

Absolutely!

The result will be the same in the end, so

Reviewed-by: Willian Rampazzo <willianr@redhat.com>


> >
> >> +        return s
> >> +
> >>      def _drain_fn(self) -> None:
> >>          """Drains the socket and runs while the socket is open."""
> >>          while self._open:
> >> --
> >> 2.20.1
> >>
> >>
> >
> >
>
>