Re: [PATCH v6 7/9] iotests: ignore import warnings from pylint

[John Snow]

On 3/3/20 7:02 PM, Philippe Mathieu-Daudé wrote:
> On 3/3/20 8:57 PM, John Snow wrote:
>> On 2/27/20 9:14 AM, Philippe Mathieu-Daudé wrote:
>>> On 2/27/20 1:06 AM, John Snow wrote:
>>>> The right way to solve this is to come up with a virtual environment
>>>> infrastructure that sets all the paths correctly, and/or to create
>>>> installable python modules that can be imported normally.
>>>>
>>>> That's hard, so just silence this error for now.
>>>
>>> I'm tempted to NAck this and require an "installable python module"...
>>>
>>> Let's discuss why it is that hard!
>>>
>>
>> I've been tricked into this before. It's not work I am interested in
>> doing right now; it's WAY beyond the scope of what I am doing here.
>>
>> It involves properly factoring all of our python code, deciding which
>> portions are meant to be installed separately from QEMU itself, coming
>> up with a versioning scheme, packaging code, and moving code around in
>> many places.
>>
>> Then it involves coming up with tooling and infrastructure for creating
>> virtual environments, installing the right packages to it, and using it
>> to run our python tests.
>>
>> No, that's way too invasive. I'm not doing it and I will scream loudly
>> if you make me.
>>
>> A less invasive hack involves setting the python import path in a
>> consolidated spot so that python knows where it can import from. This
>> works, but might break the ability to run such tests as one-offs without
>> executing the environment setup.
>>
>> Again, not work I care to do right now and so I won't. The benefit of
>> these patches is to provide some minimum viable CI CQA for Python where
>> we had none before, NOT fix every possible Python problem in one shot.
> 
> OK I guess we misunderstood each other :)
> 
> I didn't understood your comment as personal to you for this patch, but
> generic. It makes sense it is not your priority and it is obvious this
> task will take a single developer a lot of time resources. I am
> certainly NOT asking you to do it.
> 
> My question was rather community-oriented.
> (Cc'ing Eduardo because we talked about this after the last KVM forum).
> 

OK, *that* is a fair question -- but you did threaten to reject the
patch, implying it was in-scope for this series. In no uncertain terms,
it is not.


HOWEVER ... since we're here, let's discuss python packaging.

This is the iotests code base. It is not intended to be packaged as
such, it's intended to be run in-tree or in the build folder. It will
rely on files being in specific paths and so on.

(I don't remember if iotests is designed to detect features at runtime
or if it still uses the build options to skip tests. Cleber would know
better, as he's battled this distinction in the past.)

Regardless, I think iotests depends on the qemu version AND the build
configuration, so iotests shouldn't be independently packaged or
packagable. It should remain "a collection of scripts" instead.

In this paradigm, python can only find descendant files. Importing from
subfolders using relative paths.

In this case, we're trying to climb *up* the path tree, which causes
grief. One often quoted solution is to use syspath hacking to
dynamically, at runtime, add parent folders to the PYTHONPATH.

Most static analysis tooling I have used to date is unable to cope with
this workaround -- hence pylint's failure to find the package being
imported.

(If we progress to using mypy, mypy will also be unable to cope with
this statement. It becomes clear we need to create a well defined
environment so tools know where they are allowed to look for sources.)

However, the thing we are trying to import is something that arguably
can be turned into an installable package as a light Python SDK for
interfacing with QEMU. That's worth doing. This code does not (AFAIK)
depend on any build configuration. That's good!

So there are a few approaches, and they're not mutually exclusive.
Option 1 *can* be a stopgap to option 2.

1. Use the iotest runners to set the PYTHONPATH to include the
python/qemu source tree directory. The imports will fail outside of the
test runner now, but we won't have to do any syspath hacking. This is
likely not important because the tests already require a good number of
environment variables set to function properly anyway.

This is a good workaround, but still outside of the scope for this series.

Notably, any pylint gating will need to occur under this specialized
environment (with PYTHONPATH set.) This might be more of a burden at times.

2. Convert the "python/qemu" folder into an installable "qemu" module.
The version of this package can track the `git describe` version well.

Once it is installable, there are several ways to use it:

A: Install it using pip/setuptools to the system. (pip3 install .)

B: Install it to the user's environment (pip3 install --user .)

C: Create a virtual environment (using whichever virtual environment
tool of your choice) and then having entered the venv, installing it:
(pip3 install .)

D: Any of the options above, but in editable/develop mode. In this mode,
the package is installed using symlinks that point back to the source
code files. This allows you to edit the package as you work without
having to reinstall after each change.

You'll use something like this:

pip3 install [--user] -e .
python3 setup.py develop [--user]

E: Continue just setting PYTHONPATH to point to the package when needed.
This is essentially editable/develop mode but without invoking setuptools.



At the moment, I don't believe we use any virtualenv configurations or
declare our dependencies natively to python tooling -- we rely on
configure to check for distribution packages.

We may want to get serious about python and begin using
virtualenvironments in places where it makes sense (instead of
PYTHONPATH hacking), but they are expensive to build so they should be
persistent so they can be re-engaged quickly to run tests and other scripts.

(A development venv might include our qemu package, the sphinx packages
locked to the correct versions, etc. We have not had major problems here
yet, but as our usage of python expands for document building and we
revamp QAPI, it may become more important to start fashioning these
bespoke environments to create more reproducible builds across different
environments.)

--js