On Mon, Feb 15, 2021 at 04:32:29PM -0500, John Snow wrote: > On 2/11/21 9:52 PM, Cleber Rosa wrote: > > On Thu, Feb 11, 2021 at 01:58:32PM -0500, John Snow wrote: > > > This series factors the python/qemu directory as an installable > > > package. It does not yet actually change the mechanics of how any other > > > python source in the tree actually consumes it (yet), beyond the import > > > path. (some import statements change in a few places.) > > > > > > git: https://gitlab.com/jsnow/qemu/-/commits/python-package-mk3 > > > CI: https://gitlab.com/jsnow/qemu/-/pipelines/254940717 > > > (New CI job: https://gitlab.com/jsnow/qemu/-/jobs/1024230604) > > > > > > The primary motivation of this series is primarily to formalize our > > > dependencies on mypy, flake8, isort, and pylint alongside versions that > > > are known to work. It does this using the setup.cfg and setup.py > > > files. It also adds explicitly pinned versions (using Pipfile.lock) of > > > these dependencies that should behave in a repeatable and known way for > > > developers and CI environments both. Lastly, it enables those CI checks > > > such that we can enforce Python coding quality checks via the CI tests. > > > > > > An auxiliary motivation is that this package is formatted in such a way > > > that it COULD be uploaded to https://pypi.org/project/qemu and installed > > > independently of qemu.git with `pip install qemu`, but that button > > > remains *unpushed* and this series *will not* cause any such > > > releases. We have time to debate finer points like API guarantees and > > > versioning even after this series is merged. > > > > > > Some other things this enables that might be of interest: > > > > > > With the python tooling as a proper package, you can install this > > > package in editable or production mode to a virtual environment, your > > > local user environment, or your system packages. The primary benefit of > > > this is to gain access to QMP tooling regardless of CWD, without needing > > > to battle sys.path (and confounding other python analysis tools). > > > > > > For example: when developing, you may go to qemu/python/ and run `make > > > venv` followed by `pipenv shell` to activate a virtual environment that > > > contains the qemu python packages. These packages will always reflect > > > the current version of the source files in the tree. When you are > > > finished, you can simply exit the shell (^d) to remove these packages > > > from your python environment. > > > > > > When not developing, you could install a version of this package to your > > > environment outright to gain access to the QMP and QEMUMachine classes > > > for lightweight scripting and testing by using pip: "pip install [--user] > > > ." > > > > > > TESTING THIS SERIES: > > > > > > First of all, nothing should change. Without any intervention, > > > everything should behave exactly as it was before. The only new > > > information here comes from how to interact with and run the linters > > > that will be enforcing code quality standards in this subdirectory. > > > > > > To test those, CD to qemu/python first, and then: > > > > > > 1. Try "make venv && pipenv shell" to get a venv with the package > > > installed to it in editable mode. Ctrl+d exits this venv shell. While in > > > this shell, any python script that uses "from qemu.[qmp|machine] import > > > ..." should work correctly regardless of where the script is, or what > > > your CWD is. > > > > > > > Ack here, works as expected. > > > > That's a relief! > > > > You will need Python 3.6 installed on your system to do this step. For > > > Fedora: "dnf install python3.6" will do the trick. > > > > > > > You may have explained this before, so forgive me asking again. Why > > is this dependent on a given Python version, and not a *minimum* > > Python version? Are the checkers themselves susceptible to different > > behavior depending on the Python version used? If so, any hint on the > > strategy for developing with say, Python 3.8, and then having issues > > caught only on Python 3.6? > > > > It's a good question, and I have struggled with communicating the language. > So here are a few points: > > (1) Users will not need Python 3.6 on their local systems to be able to run > the linters; they will be able to run "make check" to run it under *any* > environment -- granted they have the necessary packages. (mypy, pylint, > pytest, flake8, and isort.) See note #2 below: > > (2) `pip install [--user] .[devel]` will install the needed dependencies to > the local environment/venv regardless of python version. These dependencies > are not pinned, but do express a minimum viable version (in setup.cfg) for > each tool that I tested rigorously. > > (3) The CI system will target Python 3.6 specifically, because that is our > minimum version. This will work to prevent future features from bleeding > into the codebase, which was a notable problem when we were targeting > simultaneous 2.7 and 3.x deployments. If we were going to only run against > one version, 3.6 is the defensibly correct version to run against. If we > want to run against more, I'd say let's not let perfection be the enemy of > good enough. > > (4) I considered it important to be able to run, as much as is possible, the > *EXACT SAME* environment for tests as the CI runs. For this purpose, "make > venv-check" uses pipenv to install a pinned set of dependencies (that might > be lower than what you'd get otherwise), and uses explicitly Python 3.6. > This is to make it repeatable and simple to run a test that's as close to > what the CI runner will do as possible. This takes a lot of the thinking out > of it. > > > So, to answer you more directly: > > - 3.6 is a *minimum* for "make check". (setup.cfg) > - 3.6 is a *dependency* for "make venv-check". (Pipfile, Pipfile.lock) >
OK, this answers my question. It wasn't completely clear to me that you took the care of using Pipfile.lock with one, and only one, Python version (via "make venv-check"). > And, as far as known version problems: > > - pylint 2.6.0 is not compatible with Python 3.9. They are working on it. > 2.6.1-dev works alright, but isn't released yet. The linters may be > unavailable to folks with 3.9-only environment. > > Workarounds: > > - Make your own venv using 3.7 or 3.8 > - Use the make venv-check entry point to use 3.6. > - Give up and push it to gitlab and see if the CI test passes. > > > And, finally, here's my maintainer testing strategy/promises: > > - I endeavor to make sure this package is tested and working on any non-EOL > Python version (3.6 - 3.9 at time of writing) > - I endeavor to ensure that it is easy to test and lint these files on your > local development system with minimum hassle > - Given the volatility of compatibility between different versions of > linters and python, however, the current *canonical check* is Python 3.6, > using the explicitly pinned versions in the Pipfile.lock. There may be times > (like right now) where the local linting test may not work with your version > of Python. > > > > 2. Try "make check" while still in the shell to run the Python linters > > > using the venv built in the previous step. This will pull some packages > > > from PyPI and run pytest, which will in turn execute mypy, flake8, isort > > > and pylint with the correct arguments. > > > > > > > Works as expected. I'll provide more feedback at the specific patches. > > > > > 3. Having exited the shell from above, try "make venv-check". This will > > > create and update the venv if needed, then run 'make check' within the > > > context of that shell. It should pass as long as the above did. > > > > > > > If this makes into a documentation (or on a v5), you may just want to > > tell users to run "deactivate" instead of exiting the shell completely. > > > > It depends on how you entered the shell. Literally "pipenv shell" does > create a new shell process that you should exit from. Since I suggested > using the pipenv invocation, I match that suggestion by telling the user to > exit (instead of deactivate). > > You know too much about python for your own good! > May be the exact opposite! It's that I'm much more used to plain venvs and those will not create a shell, and exiting will quit a session, which is very annoying! > > > 4. Still outside of the venv, you may try running "make check". This > > > will not install anything, but unless you have the right Python > > > dependencies installed, these tests may fail for you. You might try > > > using "pip install --user .[devel]" to install the development packages > > > needed to run the tests successfully to your local user's python > > > environment. Once done, you will probably want to "pip uninstall qemu" > > > to remove that package to avoid it interfering with other things. > > > > > > > This is good info for completeness, but I wonder if "make check" > > should exist at all. If it's a requirement for "make check-venv", the > > question becomes if it should be advertised. Hint: I don't think it > > should, it just adds some a bit of confusion IMO. > > > > I think it's cleanly separated... If you understand much about how python > virtual environments work. > > - You can run the tests on any environment you want! or, > - You can run those tests on a very, very specific environment that is > controlled with a militaristic, iron fist. > > current belief: People who are not developing python can just wait for the > little orb to turn green in Gitlab-CI and not worry about running any > particular test at all, actually. This current patch-set does not integrate > these tests into "make check" at all, on purpose. > > People who ARE developing this package (infrequently) will need to know > which they want to run. My suggestion is to use "make venv-check" as the > best predictor of the CI check, though it can be slow and clunky. > > If you develop on this package a *lot*, and you regularly use a venv to > develop, "make check" starts to make a lot more sense. > > "make" with no arguments produces the help message; > > ``` > python packaging help: > > make venv: Create pipenv's virtual environment. > NOTE: Requires Python 3.6 and pipenv. > Will download packages from PyPI. > HINT: On Fedora: 'sudo dnf install python36 pipenv' > > make venv-check: run linters using pipenv's virtual environment. > > make check: run linters using the current environment. > Hint: Install deps with: 'pip install ".[devel]"' > ``` > > ... Which, I suppose if you don't use python much, it might not make sense > to you which environment you want to run the tests under. I can probably add > a hint: > > "HINT: Run this test if you're unsure which to run." > Yeah, the "make" help message is useful as it is, but I'd indeed include this extra bit of info. Thanks for the very detailed explanation! - Cleber.
signature.asc
Description: PGP signature