Open Stack

On 13-12-20 16:33:39, Jeremy Stanley wrote:
> On 2020-12-13 14:39:58 +0100 (+0100), Luigi Toscano wrote:
> > On Saturday, 12 December 2020 00:12:36 CET Jeremy Stanley wrote:
> > > On 2020-12-11 20:38:30 +0000 (+0000), Sorin Sbarnea wrote:
> > > [...]
> > > > Regarding decoupling linting from test-requirements: yes! This was
> > > > already done by some when conflicts appeared. For old branches I
> > > > personally do not care much even if maintainers decide to disable
> > > > linting, their main benefit is on main branches.
> > > [...]
> > > 
> > > To be honest, if I had my way, test-requirements.txt files would die
> > > in a fire. Sure it's a little more work to be specific about the
> > > individual requirements for each of your testenvs in tox.ini, but
> > > the payoff is that people aren't needlessly installing bandit when
> > > they run flake8 (for example). The thing we got into the PTI about
> > > using a separate doc/requirements.txt is a nice compromise in that
> > > direction, at least.
> > 
> > Wouldn't this mean tracking requirements into two different kind
> > of places:the main requirements.txt file, which is still going to
> > be needed even for tests, and the tox environment definitions?
> 
> Technically we already do. The requirements.txt file contains actual
> runtime Python dependencies of the software (technically
> setup_requires in Setuptools parlance). Then we have this vague
> test-requirements.txt file which installs everything under the sun
> a test might want, including the kitchen sink. Tox doesn't reuse one
> virtualenv for multiple testenv definitions, it creates a separate
> one for each, so for example...

That isn't technically true within Nova, multiple tox envs use the
{toxworkdir}/shared envdir for the virtualenv.

mypy, pep8, fast8, genconfig, genpolicy, cover, debug and bandit.
 
> In the nova repo, if you `tox -e bandit` or `tox -e pep8` it's going
> to install coverage, psycopg2, PyMySQL, requests,
> python-barbicanclient, python-ironicclient, and a whole host of
> other stuff, including the entire transitive dependency set for
> everything in there, rather than just the one tool it needs to run.

Yup that's pointless.

> I can't even run the pep8 testenv locally because to do that I
> apparently need a Python package named zVMCloudConnector which wants
> root access to create files like
> /lib/systemd/system/sdkserver.service and
> /etc/sudoers.d/sudoers-zvmsdk and /var/lib/zvmsdk/* and
> /etc/zvmsdk/* in my system. WHAT?!? Do nova's developers actually
> ever run any of this themselves?

...

Which version of that package is the pep8 env pulling in for you?

I don't see any such issues with zVMCloudConnector==1.4.1 locally on
Fedora 33, tox 3.19.0, pip 20.2.2 etc.

Would you mind writing up a launchpad bug for this?

> Okay, so that one's actually in requirements.txt (might be a good
> candidate for a separate extras in the setup.cfg instead), but
> seriously, it's trying to install 182 packages (present count on
> master) just to do a "quick" style check, and the resulting .tox
> created from that is 319MB in size. How is that in any way sane? If
> I tweak the testenv:pep8 definition in tox.ini to set
> deps=flake8,hacking,mypy and and usedevelop=False, and set
> skipsdist=True in the general tox section, it installs a total of 9
> packages for a 36MB .tox directory. It's an extreme example, sure,
> but remember this is also happening in CI for each patch uploaded,
> and this setup cost is incurred every time in that context.

EWww yeah this is awful.
 
> This is already solved in a few places in the nova repo, in
> different ways. One is the docs testenv, which installs
> doc/requirements.txt (currently 10 mostly Sphinx-related entries)
> instead of combining all that into test-requirements.txt too.
> Another is the osprofiler extra in setup.cfg allowing you to `pip
> install nova[osprofiler]` to get that specific dependency. Yet still
> another is the bindep testenv, which explicitly declares deps=bindep
> and so installs absolutely nothing else (save bindep's own
> dependencies)... or, well, it would except skipsdist got set to
> False by https://review.openstack.org/622972 making that testenv
> effectively pointless because now `tox -e bindep` has to install
> nova before it can tell you what packages you're missing to be able
> to install nova. *sigh*
> 
> So anyway, there's a lot of opportunity for improvement, and that's
> just in nova, I'm sure there are similar situations throughout many
> of our projects. Using a test-requirements.txt file as a dumping
> ground for every last package any tox testenv could want may be
> convenient for tracking things, but it's far from convenient to
> actually use. The main thing we risk losing is that the
> requirements-check job currently reports whether entries in
> test-requirements.txt are compatible with the global
> upper-constraints.txt in openstack/requirements, so extending that
> to check dependencies declared in tox.ini or in package extras or
> additional external requirements lists would be needed if we wanted
> to preserve that capability.

Gibi, should we track all of this in a few launchpad bugs for Nova?

Cheers,

-- 
Lee Yarwood                 A5D1 9385 88CB 7E5F BE64  6618 BCA6 6E33 F672 2D76
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.openstack.org/pipermail/openstack-discuss/attachments/20201214/31cd9ca8/attachment-0001.sig>