opendevreview | Tim Burke proposed openstack/pbr master: Fix tempest-full job https://review.opendev.org/c/openstack/pbr/+/906072 | 00:01 |
---|---|---|
opendevreview | Tim Burke proposed openstack/pbr master: Fix tempest-full job https://review.opendev.org/c/openstack/pbr/+/906072 | 05:20 |
opendevreview | Merged openstack/ci-log-processing master: Add information about CloudFormation status on AWS https://review.opendev.org/c/openstack/ci-log-processing/+/905244 | 08:50 |
*** tosky_ is now known as tosky | 12:50 | |
fungi | clarkb: timburke has been hitting some random enoent errors on unit test/coverage jobs with https://review.opendev.org/c/openstack/pbr/+/906072 which look like they could be build backend contention from concurrent tests. is that a likely theory? could this be due to us avoiding build isolation? | 20:04 |
fungi | i suppose pbr is somewhat unique across our projects insofar as building wheels during tests rather than before they're run | 20:05 |
clarkb | is build/ a global dir rather than relative to the package? I guess even then its possible we use the asme fake package for multiple tests but I think we don't do that | 20:09 |
clarkb | we're building a wheel of pbr proper there and not a test package so that would be relative to the pbr top level dir and if multiple things interact with that then ya I suppose the build dir could be fought over | 20:09 |
fungi | yeah, relative to the git worktree root | 20:10 |
clarkb | fungi: do you know if iti s possible to configure the build dir? then we could use a temp dir fixture and pass that to whatever is building | 20:10 |
fungi | that's what i was going to look into | 20:10 |
fungi | unfortunately it's a bit of spaghetti. i guess it's setuptools as a pep-517 build backend that we're trying to change the behavior of? | 20:13 |
clarkb | yes I think so | 20:14 |
fungi | i wonder if it's setuptools or pip that decides where the build dir should be | 20:14 |
clarkb | looking at the test we create a fake package that uses pbr + setuptools to do a pep517 install | 20:15 |
clarkb | we create a venv then attempt to install that fake package into the venv. What isn't clear to me is where we are installing pbr from source yet | 20:15 |
fungi | looks like it's only the TestPEP517Support class that explicitly does build --no-isolation | 20:16 |
clarkb | ok I think it is failing in the venv setup | 20:17 |
fungi | but we saw the problem crop up in, for example, TestRequirementParsing | 20:17 |
clarkb | and that is why it races because other tests use venvs too and they all install pbr from source so will fight over the build dir | 20:17 |
clarkb | this is unrelated to the --no-isolation thing I don't think we get that far | 20:17 |
clarkb | fungi: https://opendev.org/openstack/pbr/src/branch/master/pbr/tests/test_packaging.py#L192 I think we need to pass the pip build dir flag here | 20:20 |
fungi | yeah, i guess looking at https://07b7bd876385a3d88390-6cf42e12c0a48d9a5bbf6a6c8b28b9dc.ssl.cf2.rackcdn.com/906072/2/gate/openstack-tox-cover/b7bcfba/testr_results.html for example it's happening during mkvenv-reqParse-stderr so i guess that is the venv creation after all? | 20:20 |
clarkb | ya the traceback shows it failing on line 204 of the file I linked | 20:20 |
clarkb | which is the venv creation | 20:21 |
fungi | er, the output is coming from mkvenv-reqParse-stderr i mean | 20:21 |
fungi | which i guess is making a venv for the reqParse test in that example | 20:21 |
clarkb | yes it is a venv that we can install the actual test package into | 20:21 |
clarkb | for various different tests | 20:21 |
fungi | ah, yep i should have been looking closer at the actual traceback | 20:22 |
clarkb | and since we want to test the version of pbr speculatively we have to install pbr from source into that venv otherwise we'll get it from pypi | 20:22 |
clarkb | another way to avoid this would be to create a wheel before the test suite runs then install that wheel | 20:22 |
clarkb | rather than rebuilding it | 20:22 |
fungi | so doing `python -m pip -v install -U pip wheel build /home/zuul/src/opendev.org/openstack/pbr` in the venv once it's created, actually | 20:23 |
clarkb | I'm not finding a modern way to change the build dir. Seems it was possible then pip deprecated the ability... | 20:23 |
fungi | does it only do build isolation for pep-517 backends now or something? | 20:24 |
fungi | and yes, pre-caching the build result does sound better all around | 20:24 |
clarkb | ya the --no-build-isolation flag implies you have to be doing pep517/pep518 to get isolation | 20:25 |
clarkb | the problem with precaching the build is that it creates a dependency for running the test suite that may not be intuitive | 20:25 |
clarkb | basically forcing you to always use tox | 20:25 |
clarkb | fungi: oh if you do pip wheel you can set the dir | 20:26 |
clarkb | or is that just for the output of the final build not the build itself? | 20:26 |
fungi | clarkb: how does it let you set the dir? not seeing it in `pip wheel --help` output, though pip wheel has a --isolated option | 20:29 |
clarkb | fungi: I think I misread the -w flag. That is for the output of the wheels not the build location | 20:30 |
clarkb | also pip has tons of documentation on how to change config via files and env vars. But I can't find docs on what the config options are | 20:30 |
clarkb | I guess the config options map directly to cli options? | 20:30 |
clarkb | https://stackoverflow.com/questions/64952572/output-directories-for-python-setup-py-sdist-bdist-wheel | 20:32 |
clarkb | idea: use the `build` tool to build a wheel of pbr from source. It will do so isolated. Then have pip in that venv install the resulting wheel | 20:33 |
clarkb | each test can continue to rebuild the wheel that way and we shouldn't end up with any conflicting builds due to isolation? | 20:34 |
clarkb | oh except build will fallback to bdist_wheel | 20:37 |
clarkb | and that will do the same thing here even with build isolation? | 20:37 |
clarkb | we can make pbr a pep517 package | 20:38 |
fungi | so do something like `pip install pip wheel build && build --wheel --outdir /home/zuul/ && pip install /home/zuul/pbr*.whl` ? | 20:39 |
fungi | or use a unique tempdir for each --outdir? | 20:40 |
fungi | the venv class init seems to just build a single command for pip installing everything, so making that happen in multiple stages will take some refactoring unless you're seeing an easy spot i'm missing to slot that in | 20:53 |
clarkb | ya it will require a little refactoring but I'm not sure that `build` will change anything since `build` docs say it falls back to bdist_wheel | 21:01 |
clarkb | and since pbr isn't a pep517 package I'm pretty sure it will fallback here | 21:01 |
clarkb | and yes you need a separate tmpdir otherwise tests with venv use can still collide over he use of that dir | 21:02 |
clarkb | its a bit crazy to me that everyone is on the build isolation bandwagon now but pip remove the ability to do that for non pep517 packages | 21:02 |
clarkb | ya confirmed with a local run of `build` against pbr. It writes to build/ | 21:04 |
fungi | does pip wheel --isolation force build isolation like the description would suggest? | 21:06 |
clarkb | testing | 21:08 |
clarkb | removing build/bdist.linux-x86_64/wheel | 21:09 |
clarkb | doesn't appear to | 21:09 |
clarkb | I think all of the isoalted flags only matter if pep517 is involved | 21:09 |
clarkb | ah it installed setuptools and wheel into a new venv | 21:09 |
clarkb | but the build of the wheel itself used bdist_wheel which isn't aware | 21:09 |
clarkb | crazy idea maybe: what if we just run the test suite serially. I don't think it takes veryl ong for pbr. We can slap a todo on there with a pointer to the inter test conflict and worry about it another day | 21:11 |
clarkb | and really only worry if/when we have enough tests where throughput becomes an issue | 21:11 |
clarkb | side note: if the build dir is already gone and you are trying to delete it then maybe we don't need to worry so much about it | 21:12 |
clarkb | but that is a likely won't fix in setuptools since they want everyone using pep517 | 21:12 |
opendevreview | Merged openstack/pbr master: Fix tempest-full job https://review.opendev.org/c/openstack/pbr/+/906072 | 21:43 |
fungi | clarkb: yeah, that's what timburke suggested in 906072 | 22:00 |
fungi | i agree it's the simplest way around this, but feels like using a sledge hammer to turn a screw | 22:00 |
fungi | but yeah, if the unit tests for pbr are fast regardless, i don't suppose parallel tests are that important | 22:01 |
clarkb | ya I think in this case because we actually care about behavior under tools like pip and their fallbacks to bdist_wheel (though this case is a bit of a side effect) not working too hard aroudn those tools seems reaosnable | 22:02 |
clarkb | at the same time I'm just amazed the tools don't have any method of working around this | 22:02 |
fungi | wfm | 22:02 |
clarkb | makes me wonder what changed for them to go full steam ahead on the isolation. I guess enough years of dealing with the old system | 22:02 |
fungi | low change volume for pbr plus pip &co slowly cutting off functionality for anything that isn't pep 571 | 22:03 |
fungi | 517 | 22:03 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!