Hi, Thanks for the detailed feedback! I will look at making these changes.
On Fri, 10 Jul 2020 at 15:20, John Snow <js...@redhat.com> wrote: > > > > On 7/7/20 3:08 AM, Alex Bennée wrote: > > From: Robert Foley <robert.fo...@linaro.org> > > > <snip> > > + def recv(self, n=1, sleep_delay_s=0.1): > > + """Return chars from in memory buffer""" > > + start_time = time.time() > > + while len(self._buffer) < n: > > + time.sleep(sleep_delay_s) > > + elapsed_sec = time.time() - start_time > > + if elapsed_sec > self._recv_timeout_sec: > > + raise socket.timeout > > + chars = ''.join([self._buffer.popleft() for i in range(n)]) > > + # We choose to use latin1 to remain consistent with > > + # handle_read() and give back the same data as the user would > > + # receive if they were reading directly from the > > + # socket w/o our intervention. > > + return chars.encode("latin1") > > + > > console_socket.py:89:4: W0221: Parameters differ from overridden 'recv' > method (arguments-differ) > > Seems pretty different from the asyncore.dispatcher recv method, is that > intentional? The intention is that the API be the same as asyncore.dispatcher recv. The sleep_delay_s can be removed, and n is the same as buffer_size in asyncore.dispatcher recv. Will plan to rename n -> buffer_size. > https://github.com/python/cpython/blob/master/Lib/asyncore.py > <snip> > > def __enter__(self): > > return self > > @@ -580,7 +591,11 @@ class QEMUMachine: > > Returns a socket connected to the console > > """ > > if self._console_socket is None: > > - self._console_socket = socket.socket(socket.AF_UNIX, > > - socket.SOCK_STREAM) > > - self._console_socket.connect(self._console_address) > > + if self._drain_console: > > + self._console_socket = ConsoleSocket(self._console_address, > > + > > file=self._console_log_path) > > Needs one more space, but the line is already too long as-is. > > > + else: > > + self._console_socket = socket.socket(socket.AF_UNIX, > > + socket.SOCK_STREAM) > > + self._console_socket.connect(self._console_address) > > return self._console_socket > > > > This makes the typing for _console_socket really tough ... but > technically not a regression as the mypy code isn't merged yet. >From the comment on mypy, I understand that we need to return a constant type? One option to provide a constant type is to simply always return ConsoleSocket here. A few changes would be needed inside of ConsoleSocket, but essentially ConsoleSocket would handle the detail of draining the console (or not), and thus eliminate this if/else above reducing it to something like this: self._console_socket = ConsoleSocket(self._console_address, file=self._console_log_path, drain=self._drain_console) How does this sound? Thanks & Regards, -Rob > > --js >