Re: [PATCH v2 00/16] python: add mypy support to python/qemu

2020-06-04 Thread Kevin Wolf
Am 02.06.2020 um 23:45 hat John Snow geschrieben:
> Requires: 20200602194844.15258-1-js...@redhat.com
> 
> This series is extracted from my larger series that attempted to bundle
> our python module as an installable module. These fixes don't require that,
> so they are being sent first as I think there's less up for debate in here.
> 
> This requires my "refactor shutdown" patch as a pre-requisite.

I had only minor comments, so with or without the suggested changes:

Reviewed-by: Kevin Wolf 




Re: [PATCH v2 00/16] python: add mypy support to python/qemu

2020-06-03 Thread John Snow



On 6/3/20 4:24 AM, Vladimir Sementsov-Ogievskiy wrote:
> 03.06.2020 00:45, John Snow wrote:
>> Requires: 20200602194844.15258-1-js...@redhat.com
> 
> Hmm, somehow, I can't find it neither in
> https://lists.gnu.org/archive/html/qemu-devel/
> nor in my thunderbird..
> 
> Could you post sequence of your series by subject, or export a git branch?
> 
> 

1.
[PATCH v2 0/1] python/machine.py: refactor shutdown​
https://github.com/jnsnow/qemu/tree/python-package-reorder
(Patch that re-writes shutdown and kill)

2.
[PATCH v2 00/16] python: add mypy support to python/qemu​
https://github.com/jnsnow/qemu/tree/python-package-mypy
(This patchset.)

3.
[PATCH 0/7] python: create installable package
https://github.com/jnsnow/qemu/tree/python-package-refactor
(Python package series)




Re: [PATCH v2 00/16] python: add mypy support to python/qemu

2020-06-03 Thread Vladimir Sementsov-Ogievskiy

03.06.2020 00:45, John Snow wrote:

Requires: 20200602194844.15258-1-js...@redhat.com


Hmm, somehow, I can't find it neither in 
https://lists.gnu.org/archive/html/qemu-devel/
nor in my thunderbird..

Could you post sequence of your series by subject, or export a git branch?


--
Best regards,
Vladimir



Re: [PATCH v2 00/16] python: add mypy support to python/qemu

2020-06-02 Thread John Snow



On 6/2/20 5:51 PM, Eric Blake wrote:
> On 6/2/20 4:45 PM, John Snow wrote:
>> Requires: 20200602194844.15258-1-js...@redhat.com
> 
> I don't know if patchew understand that, or if it requires:
> 
> Based-on: 20200602194844.15258-1-js...@redhat.com
> 
>>
>> This series is extracted from my larger series that attempted to bundle
>> our python module as an installable module. These fixes don't require
>> that,
>> so they are being sent first as I think there's less up for debate in
>> here.
>>
>> This requires my "refactor shutdown" patch as a pre-requisite.
>>
>> "v2":
>> - This version supports iotests 297
>> - Many patches merged by Phil are removed
>> - Replaces iotests.py type aliases with centralized ones
>>    (See patch 2)
>> - Imports etc are reworked to use the non-installable
>>    package layout instead. (Mostly important for patch 3)
>>
>> Testing this out:
>> - You'll need Python3.6+
>> - I encourage you to use a virtual environment!
>> - You don't necessarily need these exact versions, but I didn't test the
>>    lower bounds, use older versions at your peril:
>>    - pylint==2.5.0
>>    - mypy=0.770
>>    - flake8=3.7.8
>>
>>> cd ~/src/qemu/python/
>>> flake8 qemu
>>> mypy --strict qemu
>>> cd qemu
>>> pylint *.py
>>
>> These should all 100% pass.
>>
>> ---
>>
>> Open RFC: What's the right way to hook this into make check to prevent
>> regressions?
> 
> We recently added iotest 297 in group meta; is there a way to run
> './check -g meta' alongside the other iotests that 'make check' already
> triggers?
> 

If we want to distribute any of this code independently of qemu.git (And
I think we do), I think relying on the iotests infrastructure will hurt
more than it will help.

I think I should try to maintain some independence of this folder from
the rest of the QEMU base; so it should be able to run the linting tests
under its own power.

(So, I guess, a Makefile?)

but there's further problems: this infrastructure is 3.6+ only, but the
build system only requires 3.5+. It has to be an optional testing target
that executes only when it's possible to. It also requires additional
dependencies not checked for in configure -- mypy, pylint, and flake8.

I am wondering if there's a nice way to create a check target that
builds a virtual environment with pinned dependencies, and then uses
that to run the lint tests. As long as the machine you're running on has
at least python3.6+ it should be able to run the tests.

I just don't really have a plan yet...

--js




Re: [PATCH v2 00/16] python: add mypy support to python/qemu

2020-06-02 Thread Eric Blake

On 6/2/20 4:45 PM, John Snow wrote:

Requires: 20200602194844.15258-1-js...@redhat.com


I don't know if patchew understand that, or if it requires:

Based-on: 20200602194844.15258-1-js...@redhat.com



This series is extracted from my larger series that attempted to bundle
our python module as an installable module. These fixes don't require that,
so they are being sent first as I think there's less up for debate in here.

This requires my "refactor shutdown" patch as a pre-requisite.

"v2":
- This version supports iotests 297
- Many patches merged by Phil are removed
- Replaces iotests.py type aliases with centralized ones
   (See patch 2)
- Imports etc are reworked to use the non-installable
   package layout instead. (Mostly important for patch 3)

Testing this out:
- You'll need Python3.6+
- I encourage you to use a virtual environment!
- You don't necessarily need these exact versions, but I didn't test the
   lower bounds, use older versions at your peril:
   - pylint==2.5.0
   - mypy=0.770
   - flake8=3.7.8


cd ~/src/qemu/python/
flake8 qemu
mypy --strict qemu
cd qemu
pylint *.py


These should all 100% pass.

---

Open RFC: What's the right way to hook this into make check to prevent
regressions?


We recently added iotest 297 in group meta; is there a way to run 
'./check -g meta' alongside the other iotests that 'make check' already 
triggers?


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org