| Message ID | 20201006235817.3280413-1-jsnow@redhat.com |
|---|---|
| Headers | show |
| Series | python/qemu: strictly typed mypy conversion, pt2 | expand |
Am 07.10.2020 um 01:57 hat John Snow geschrieben: > Borrowed from the QAPI cleanup series, use the same configuration to > standardize the way we write and sort imports. > > Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Am 07.10.2020 um 01:58 hat John Snow geschrieben: > These should all be purely annotations with no changes in behavior at > all. You need to be in the python folder, but you should be able to > confirm that these annotations are correct (or at least self-consistent) > by running `mypy --strict qemu`. > > Signed-off-by: John Snow <jsnow@redhat.com> 'mypy --strict qemu' doesn't have clean output after this patch because ConsoleSocket isn't converted yet. But then, technically the commit message doesn't make this claim, and you can indeed check the self-consistency when you ignore the ConsoleSocket related errors, so probably not a problem. Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Am 07.10.2020 um 01:58 hat John Snow geschrieben: > Finish the typing of console_socket.py with annotations and no code > changes. > > Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Am 07.10.2020 um 01:58 hat John Snow geschrieben: > We can work directly in bytes instead of translating back and forth to > string, which removes the question of which encodings to use. > > Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
On 10/7/20 5:53 AM, Kevin Wolf wrote: > Am 07.10.2020 um 01:58 hat John Snow geschrieben: >> Like many other Optional[] types, it's not always a given that this >> object will be set. Wrap it in a type-shim that raises a meaningful >> error and will always return a concrete type. >> >> Signed-off-by: John Snow <jsnow@redhat.com> > >> @@ -515,11 +515,13 @@ def set_qmp_monitor(self, enabled=True): >> line. Default is True. >> @note: call this function before launch(). >> """ >> - if enabled: >> - self._qmp_set = True >> - else: >> - self._qmp_set = False >> - self._qmp = None >> + self._qmp_set = enabled > > This change seems unrelated to wrapping the connection in a property. > Intuitively, it makes sense that the connection of a running instance > doesn't go away just because I disable QMP in the command line for the > next launch. > > If this is the reasoning behind the change, maybe mention it in the > commit message. > > With this: > Reviewed-by: Kevin Wolf <kwolf@redhat.com> > Oh, yes. That's what happened here -- and it got folded in here specifically to make that access check consistent. I'll update the commit message.
On 10/7/20 6:46 AM, Kevin Wolf wrote: > Am 07.10.2020 um 01:58 hat John Snow geschrieben: >> These should all be purely annotations with no changes in behavior at >> all. You need to be in the python folder, but you should be able to >> confirm that these annotations are correct (or at least self-consistent) >> by running `mypy --strict qemu`. >> >> Signed-off-by: John Snow <jsnow@redhat.com> > > 'mypy --strict qemu' doesn't have clean output after this patch because > ConsoleSocket isn't converted yet. But then, technically the commit > message doesn't make this claim, and you can indeed check the > self-consistency when you ignore the ConsoleSocket related errors, so > probably not a problem. > > Reviewed-by: Kevin Wolf <kwolf@redhat.com> > Whoops, old commit message. I decided against folding in the new changes because they are newer and have been through the review wringer a lot less. I'll fix this commit message up.
On 10/7/20 6:59 AM, Kevin Wolf wrote: > Am 07.10.2020 um 01:58 hat John Snow geschrieben: >> The type and parameter names of recv() should match socket.socket(). > > Should this be socket.socket without parentheses (the class name)? > socket.socket() is the constructor and it takes very different > parameters. > You're right. >> OK, easy enough, but in the cases we don't pass straight through to the >> real socket implementation, we probably can't accept such flags. OK, for >> now, assert that we don't receive flags in such cases. >> >> Signed-off-by: John Snow <jsnow@redhat.com> > > Reviewed-by: Kevin Wolf <kwolf@redhat.com> > Thanks!
On 10/7/20 7:35 AM, Kevin Wolf wrote: > Am 07.10.2020 um 01:58 hat John Snow geschrieben: >> Formalize the options used for checking the python library. You can run >> mypy from the directory that mypy.ini is in by typing `mypy qemu/`. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> python/mypy.ini | 4 ++++ >> 1 file changed, 4 insertions(+) >> create mode 100644 python/mypy.ini >> >> diff --git a/python/mypy.ini b/python/mypy.ini >> new file mode 100644 >> index 00000000000..7a70eca47c6 >> --- /dev/null >> +++ b/python/mypy.ini >> @@ -0,0 +1,4 @@ >> +[mypy] >> +strict = True > > $ mypy --strict qemu > mypy.ini: [mypy]: Strict mode is not supported in configuration files: specify individual flags instead (see 'mypy -h' for the list of flags enabled in strict mode) > Success: no issues found in 6 source files > $ mypy --version > mypy 0.740 > > Did this change in newer mypy versions? I guess it's time that I get the > new laptop which will involve installing a newer Fedora release. :-) > >> +python_version = 3.6 >> +warn_unused_configs = True >> \ No newline at end of file > > Kevin > 0.770 lets you use strict in the config file. Fairly modern. I intend to use this version in the CI venv that I am cooking up to check these, so no need to hurry and update your fedora. 'pip3 install --user mypy>=0.770' should work out just fine until then. Maybe I should drop back down to >=0.730, but I liked being able to force the stricter options in the conf file directly. I also liked the idea that if new strict options got added in the future, we'd acquire them automatically. I felt like anything we disabled should be a conscious and explicit choice, instead of the opposite. --js
On 10/6/20 7:57 PM, John Snow wrote: > Continuing where I left off prior to the 5.1 release, this series > touches up a few odds and ends and introduces mypy hints. > > What's new: > > - Using isort to solidify import order > - Patches adding small corrections and typing for console_socket > - A few error class changes for qmp.py > > Like my QAPI series, this requires: > > - pylint >= 2.6.0 > - flake8 >= 3.8.0 > - mypy >= 0.770 > - isort >= 4.3.0 (Presumably...) > > What this series doesn't do: > > - Create a proper python package > - Establish a CI test to prevent regressions > - Fix the docstring conventions in the library > > Those are coming soon! (and in the order mentioned.) > > Changes against the last version of this series that was sent prior to > 5.1: > > 001/20:[down] 'python/qemu: use isort to lay out imports' > 002/20:[0005] [FC] 'python/machine.py: Fix monitor address typing' > 003/20:[0015] [FC] 'python/machine.py: reorder __init__' > 004/20:[0009] [FC] 'python/machine.py: Don't modify state in _base_args()' > 005/20:[0002] [FC] 'python/machine.py: Handle None events in events_wait' > 006/20:[0006] [FC] 'python/machine.py: use qmp.command' > 007/20:[----] [-C] 'python/machine.py: Add _qmp access shim' > 008/20:[----] [-C] 'python/machine.py: fix _popen access' > 009/20:[0006] [FC] 'python/qemu: make 'args' style arguments immutable' > 010/20:[----] [--] 'iotests.py: Adjust HMP kwargs typing' > 011/20:[0010] [FC] 'python/qemu: Add mypy type annotations' > 012/20:[down] 'python/qemu/console_socket.py: Correct type of recv()' > 013/20:[down] 'python/qemu/console_socket.py: fix typing of settimeout' > 014/20:[down] 'python/qemu/console_socket.py: Clarify type of drain_thread' > 015/20:[down] 'python/qemu/console_socket.py: Add type hint annotations' > 016/20:[down] 'python/console_socket: avoid encoding to/from string' > 017/20:[down] 'python/qemu/qmp.py: Preserve error context on re-raise' > 018/20:[down] 'python/qemu/qmp.py: re-raise OSError when encountered' > 019/20:[down] 'python/qemu/qmp.py: Straighten out exception hierarchy' > 020/20:[down] 'python: add mypy config' > > 02: import order differences, context changes from console_socket.py > 03: (minor) changes for console_socket, RB-s dropped just in case > 04: import order differences > 05: import order differences > 06: import order differences > 09: import order differences > 11: import order differences, small changes for console_socket > > John Snow (20): > python/qemu: use isort to lay out imports > python/machine.py: Fix monitor address typing > python/machine.py: reorder __init__ > python/machine.py: Don't modify state in _base_args() > python/machine.py: Handle None events in events_wait > python/machine.py: use qmp.command > python/machine.py: Add _qmp access shim > python/machine.py: fix _popen access > python/qemu: make 'args' style arguments immutable > iotests.py: Adjust HMP kwargs typing > python/qemu: Add mypy type annotations > python/qemu/console_socket.py: Correct type of recv() > python/qemu/console_socket.py: fix typing of settimeout > python/qemu/console_socket.py: Clarify type of drain_thread > python/qemu/console_socket.py: Add type hint annotations > python/console_socket: avoid encoding to/from string > python/qemu/qmp.py: Preserve error context on re-raise > python/qemu/qmp.py: re-raise OSError when encountered > python/qemu/qmp.py: Straighten out exception hierarchy > python: add mypy config > > python/mypy.ini | 4 + > python/qemu/.isort.cfg | 7 + > python/qemu/accel.py | 9 +- > python/qemu/console_socket.py | 54 +++--- > python/qemu/machine.py | 306 +++++++++++++++++++++------------- > python/qemu/qmp.py | 114 ++++++++----- > python/qemu/qtest.py | 55 +++--- > tests/qemu-iotests/iotests.py | 2 +- > 8 files changed, 331 insertions(+), 220 deletions(-) > create mode 100644 python/mypy.ini > create mode 100644 python/qemu/.isort.cfg > Thanks, I am staging patches 1-17 -- alongside the Python maintainers patch I have been kicking around -- to my python branch. Cleaning up the error classes in 18-19 is looking more fiddly, so I'm spinning it out into a new small series for better review. https://gitlab.com/jsnow/qemu.git python https://gitlab.com/jsnow/qemu/-/tree/python --js