| Message ID | 20201207200737.5090-1-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] python: add __repr__ to ConsoleSocket to aid debugging | expand |
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
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 > >
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 >> >> > >
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 > >> > >> > > > > > > <div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Em seg, 7 de dez de 2020 19:14, Philippe Mathieu-Daudé <<a href="mailto:philmd@redhat.com">philmd@redhat.com</a>> escreveu:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Willian,<br> <br> On 12/7/20 10:35 PM, Willian Rampazzo wrote:<br> > On Mon, Dec 7, 2020 at 5:10 PM Alex Bennée <<a href="mailto:alex.bennee@linaro.org" target="_blank" rel="noreferrer">alex.bennee@linaro.org</a>> wrote:<br> >><br> >> While attempting to debug some console weirdness I thought it would be<br> >> worth making it easier to see what it had inside.<br> >><br> >> Signed-off-by: Alex Bennée <<a href="mailto:alex.bennee@linaro.org" target="_blank" rel="noreferrer">alex.bennee@linaro.org</a>><br> >> ---<br> >> python/qemu/console_socket.py | 8 ++++++++<br> >> 1 file changed, 8 insertions(+)<br> >><br> >> diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py<br> >> index f060d79e06..77966d1fe9 100644<br> >> --- a/python/qemu/console_socket.py<br> >> +++ b/python/qemu/console_socket.py<br> >> @@ -45,6 +45,14 @@ class ConsoleSocket(socket.socket):<br> >> if drain:<br> >> self._drain_thread = self._thread_start()<br> >><br> >> + def __repr__(self):<br> >> + s = super(ConsoleSocket, self).__repr__()<br> >> + s = s.rstrip(">")<br> >> + s += ", logfile=%s" % (self._logfile)<br> >> + s += ", drain_thread=%s" % (self._drain_thread)<br> >> + s += ">"<br> > <br> > We could use something more pythonic for this file. Instead of 3<br> > string concatenations, my suggestion is to go with string formatting,<br> > like:<br> > <br> > s = "%s, logfile=%s, drain_thread=%s>" % (s, self._logfile, self._drain_thread)<br> > <br> > As str is immutable in Python, it avoids unnecessary copies.<br> <br> With this (and John's comment) addressed, are you OK to add<br> your R-b tag?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Absolutely!</div><div dir="auto"><br></div><div dir="auto">The result will be the same in the end, so</div><div dir="auto"><br></div><div dir="auto">Reviewed-by: Willian Rampazzo <<a href="mailto:willianr@redhat.com">willianr@redhat.com</a>></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br> > <br> >> + return s<br> >> +<br> >> def _drain_fn(self) -> None:<br> >> """Drains the socket and runs while the socket is open."""<br> >> while self._open:<br> >> --<br> >> 2.20.1<br> >><br> >><br> > <br> > <br> <br> </blockquote></div></div></div>
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
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:
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(+) -- 2.20.1