| cardoe | clarkb, fungi: which of you suggested sharing to figure out where git is used badly? https://review.opendev.org/c/openstack/ironic/+/983490 and the logs... https://zuul.opendev.org/t/openstack/build/dac830b8950a40b3832ec7641512dcb6 | 00:25 |
|---|---|---|
| cardoe | It's the stock openstack-tox-pep8 job that fails on opendev.org | 00:25 |
| tkajinam | gouthamr, yes we publish our modules to puppet forge, so that it can be easily downloaded by some tools. honestly I wasn't really aware of that "seeking contributor" status transition there, though | 03:23 |
| amorin | thank you fungi and gouthamr | 05:35 |
| opendevreview | Dmitriy Rabotyagov proposed openstack/governance master: Retire Venus Project https://review.opendev.org/c/openstack/governance/+/981959 | 06:29 |
| opendevreview | Dmitriy Rabotyagov proposed openstack/governance master: Retire Venus Project https://review.opendev.org/c/openstack/governance/+/981959 | 06:31 |
| frickler | cardoe: oh, that's for the pre-commit stuff ... well I don't want to say that I never was a fan of that ... but good luck with making that CI-ready I guess | 13:10 |
| cardoe | Well I had suggested we have a generic step that used pre-commit directly and not called it via tox because you can control it much better. | 13:11 |
| cardoe | I also personally don't use pre-commit for a number of reasons. I use prek. | 13:11 |
| cardoe | prek allows me to specify a cache and such for my CI | 13:12 |
| cardoe | You say good luck but I would say a majority of the big repos are calling it this way. | 13:13 |
| fungi | cardoe: in that example, it's not the constraints file case i was thinking of.the ironic team has decided to run precommit in their pep8 job, and configured precommit to install hacking over the internet from a git remote. inefficient and fragile decision for a ci job | 13:44 |
| fungi | oh, i see frickler also dug into it | 13:45 |
| fungi | anyway, i recommend avoiding precommit if it can't be told to install things from pypi (we provide a caching proxy) or a local git cache, or if it can do the latter then configure it to use the one zuul provides (and add the openstack/hacking repo as a required-project for ironic's variant of the pep8 job) | 13:46 |
| cardoe | fungi: well you say ironic team but that's something submitted to us from I think tkajinam and it's in a majority of the repos now days. | 13:59 |
| cardoe | fungi: and pre-commit cannot... prek can. | 13:59 |
| fungi | if that was a mass "improvement" across lots of projects, it ought to be revisited/reverted, in my opinion, until it can be made less fragile | 14:00 |
| cardoe | I mean it's just fragile cause we don't want to change our common pep8 entry point. | 14:06 |
| fungi | well, we did find other mechanisms for things like retrieving the constraints file from a local cache when provided (based on an envvar set in jobs) or over the internet when not (to support local developer workflows) | 14:37 |
| clarkb | yes, I've consistently called out this drawback of pre commit and unfortunately no one else seems to really care | 14:50 |
| clarkb | but there are good reasons to not install everything from source and instead use packages | 14:50 |
| tkajinam | we could probably use repo: local and install hooks in advance? though it removes the benefit of explicit pin of check libs and brings us the old "stable-only: pin xxx to yyy" work | 14:53 |
| tkajinam | or large amount of chance to keep using ancient hacking pin for multiple years | 14:54 |
| clarkb | tkajinam: ideally you would use what is on pypi. Has pre commit still not added support for installing from pypi? | 14:55 |
| clarkb | but also I'm not sure why using a local git repo changes what versiosn you can install | 14:56 |
| clarkb | its not like the commits are less available that way | 14:56 |
| tkajinam | yeah I'm checking that now | 14:56 |
| tkajinam | clarkb, .pre-commit-config.yaml has explicit version defined so we are not bothered by new hacking/flake8/pylint release killing stable branches | 14:57 |
| tkajinam | because these are not pinned in u-c | 14:57 |
| tkajinam | some repos have their own pin which quite often result in capping these libs to 5 years old versions | 14:57 |
| tkajinam | which caused multiple troubles when we migrated ubuntu instances to noble for example | 14:58 |
| * tkajinam is looking at pre-commit doc | 14:58 | |
| tkajinam | https://pre-commit.com/#python | 14:59 |
| tkajinam | maybe we could try repo:local with additional_dependencies ... ? | 14:59 |
| clarkb | tkajinam: the hack people use is to have a python package in a git repo that has install requirements for the things you actually want to install. When you install the git repo from source everything else gets installed from pypi | 15:00 |
| clarkb | which is crazy to think about when the installation tool can just be pointed at a package on pypi in the first place. But maybe this workaround is worth considering now | 15:01 |
| tkajinam | I agree | 15:05 |
| tkajinam | the problem with pre-commit is that it loads .pre-commit-config.yaml from the git repository to detect the hook. we can use local + additional_dependencies but have to copy the content of .pre-commit-hook.yaml into each repo... | 15:06 |
| clarkb | I still don't understand what value pre-commit provides when you can run tox -epep8 and have it run the commands that pre commit is going to run | 15:08 |
| clarkb | and tox will manage all the deps too | 15:08 |
| tkajinam | if you run only hacking checks, yes, but pre-commit allows us to easily integrate more checks | 15:09 |
| tkajinam | like the one to get rid of CRLF | 15:09 |
| * dansmith _hates_ precommit | 15:10 | |
| clarkb | sure but you can just run those same commands with tox is my point | 15:10 |
| dansmith | especially any of them that modifies (even if just removing CRLF) before commit | 15:10 |
| clarkb | pre-commit is a "install these tools and execute them engine" which si exactly what tox does | 15:10 |
| clarkb | so using them together is somewhat redudant and imo tox does a better job at the install these tools aspect. I can't weigh in on the execute them side as I haven't really compared that part | 15:10 |
| sean-k-mooney | the performance between pre-commit and tox has varied over time pre-commit used to be faster | 15:11 |
| sean-k-mooney | but im not sure if that is sticly true now | 15:12 |
| tkajinam | dansmith, if you do not integrate .pre-commit into githooks manually it doesn't be run at git commit | 15:12 |
| tkajinam | it isn't run * | 15:12 |
| dansmith | tkajinam: I understand :) | 15:12 |
| sean-k-mooney | pre-commit does cache thing locally by the way but that obviously does not work for our ci job | 15:13 |
| sean-k-mooney | since its a clean filestyem and we dont want to have ot bake in things like that into the image | 15:13 |
| clarkb | sean-k-mooney: which confuses me because they are both running the same commands presumably | 15:14 |
| tkajinam | well just make my point clear, I'm not the one who initially brought that but saw it introduced in a few oslo repos and found it useful to add more checks and did brought to a few more (and saw core ironic introduced it so covered a few ironic-owned repos not getting it) | 15:15 |
| sean-k-mooney | fungi: by the way i dont know if this will fix it but on the UC file issue causeign local tox envs to be slow/recreated on uc change i wonder if we could avoid that by moving form === to <= for upper constraits | 15:15 |
| tkajinam | s/my point/my position/ | 15:15 |
| sean-k-mooney | fungi: with our current approch if there is any change it triggers a recreate instead of only recreating on min bumps | 15:16 |
| sean-k-mooney | that was one of the regressiosn form tox 3 to tox 4 imo | 15:16 |
| sean-k-mooney | stephenfin propose a new option to avoid reinstallign the deps in a exsiting tox env partly to help with this | 15:16 |
| clarkb | sean-k-mooney: constraints must be === | 15:17 |
| sean-k-mooney | oh it does not supprot the other comparitors | 15:17 |
| sean-k-mooney | thats unfortunet | 15:17 |
| clarkb | it might parse them but I don't think the end result is as expected. AIUI constraints must avoid dep resolution | 15:17 |
| clarkb | because they win over resolution | 15:18 |
| sean-k-mooney | ok i was expecting them to be an input to the dep resolution | 15:18 |
| tkajinam | yeah it does not strictly guarantee the result, for example in case something requires much lower versions. | 15:18 |
| sean-k-mooney | that might have also changed over time with the change in pip resolver behvior | 15:19 |
| clarkb | constraints are "weird" when it comes to this stuff because they are a short circuit. They are why we generally don't have dep resolution issues and why when constraints are not used you sometimes see pip spend 10 minutes trying to figure out what to install | 15:20 |
| clarkb | I think if you don't use === you may fall into the old dep handling path that doesn't do proper resolution which always confused everyone | 15:20 |
| clarkb | also upper constraints are why we see the problems in pypi that no one else ever does. because we requier a specific version that the dep solver can't see when pypi serves ancient stale indexes. Everyoen else installs the old ancient stuff based on the dep solver solution. We fail because the expected version is completely missing | 15:21 |
| sean-k-mooney | well expected is somewhat nebuls there upper-constaits is not intended ot force the news one if the current dep fullfile sthe actula requirement | 15:22 |
| sean-k-mooney | that was the change in behviro between tox 3 an 4 i was refering too | 15:22 |
| sean-k-mooney | in tox 4 it will recret the enve if the constriant file changes even if your required deps for your project are not affected if i understand correctly | 15:23 |
| clarkb | sure. Use nox then you can decide if/when venvs are recreated more carefully :) | 15:23 |
| sean-k-mooney | ya so that kidn of what the new flag stephen propsoed to tox does | 15:23 |
| clarkb | but I think that comes down to how and when tox decides to rerun installation | 15:23 |
| sean-k-mooney | you can now say dont install the deps when invokign it on the cli | 15:24 |
| sean-k-mooney | we can also now specify constraits files in the toxini without using install commands by the way or the -c flag in deps | 15:25 |
| sean-k-mooney | https://github.com/tox-dev/tox/pull/3556 | 15:25 |
| sean-k-mooney | https://github.com/tox-dev/tox/pull/3792 | 15:29 |
| sean-k-mooney | --skip-env-install | 15:29 |
| sean-k-mooney | until we stop abusing tox for preparing venv for tempst we proably shoudl start using that | 15:29 |
| sean-k-mooney | i.e. to make sure the tempst plugins we isntalled dont get removed because uc updated | 15:30 |
| sean-k-mooney | i guess in ci that wont happen because we are geenrally using the local checkout form the requirement repo instead of the live version | 15:31 |
| sean-k-mooney | but it defintly can happen locally | 15:31 |
| sean-k-mooney | by the way pre-commit just uses pip to install pytonon tool so if we have set up the mirros in /etc/pip.conf which i belive oru configure_mirros role those it will used them | 15:42 |
| fungi | tkajinam: keep in mind that you can compose multiple pip constraints files as long as they don't contradict one another, just pass the -c option more than once | 16:05 |
| fungi | so a pip install command can use the global upper-constraints.txt from the requirements repo and also a local test-constraints.txt (or whatever name you like) from the repo being tested, for example | 16:08 |
| fungi | so projects can still constrain their independent preference for hacking/flake8/pyflakes/whatever | 16:09 |
| sean-k-mooney | that tecnially posibel but it requires a littel more disiplin to make sure that such local constartis are only for linters | 16:14 |
| sean-k-mooney | ie. we have the general rule that service project shoudl not commit local caps in there requirements file but we do allwo excludling known bad version locally | 16:15 |
| sean-k-mooney | so reviewers woudl jsut need to make sure that any local constaitn file for linter targets are just ath only for linters/packages not managed by the requiremetns repo | 16:15 |
| clarkb | its worth noting that we ran hacking with global requirements for years and it really wasn't an issue | 16:15 |
| clarkb | you just had to agree to sync up at the beginning of the cycle and fix the new rules or add ignore entries | 16:16 |
| sean-k-mooney | ya i dont actully have an issue with hacking being under the requrieemsnt process for what its worht | 16:16 |
| sean-k-mooney | i jsut know we have been bitten in the past with cap in requireemtns.txt in the past coming up to release or when tryign to bump things in uc | 16:17 |
| sean-k-mooney | they are very easy to forget to remove | 16:17 |
| cardoe | sean-k-mooney: pre-commit might not be faster but prek is. | 19:45 |
| cardoe | I can run prek in sub 1 second... tox -e pep8 takes 40 seconds. | 19:45 |
| cardoe | I'm not gonna run tox -e pep8 before pushing to Gerrit and burning a bunch of compute cycles... but I will run prek. | 19:46 |
| clarkb | cardoe: is prek not just running hacking like tox -e pep8 though? | 19:46 |
| clarkb | after the first run I would expect them to be very similar timing wise | 19:47 |
| fungi | i wouldn't be surprised if tox is burning a lot of cycles checking the state of the virtualenv is uses | 19:47 |
| cardoe | pre-commit is much slower as well and every time its run via tox it takes the slow path of checking all its virtualenvs as well. So tox is checking virtualenvs. It's checking virtualenvs. | 19:49 |
| clarkb | ah I mostly use nox these days and use the just run against the existing venv flag when I want it to be quick | 19:52 |
| clarkb | nox --no-install --reuse-existing-virtualenvs | 19:52 |
| JayF | Yeah tox has really slowed down as of late, at least in terms of the "eye test" for me. But I found that moving to a local TOX_CONSTRAINTS_FILE fixed some of that so I'm not sure it's valid data. | 21:09 |
| fungi | yes, if you're using the default constraints url in your tox.ini that's hitting a redirect on releases.opendev.org to a file on opendev.org gitea, and there may be some additional redirects on top of that, so multiple web requests to different systems any of which might be running a bit slow recently | 21:12 |
| clarkb | is tox --skip-env-install any quicker? maybe with --skip-pkg-install as well? | 21:15 |
| clarkb | for things like hacking which don't operate against the installed code but instead the source you shouldn't need to do an installation and if you already have a venv that should be faster | 21:15 |
| fungi | i used to configure tox in some of our other projects to not install the repository being tested (and therefore none of its dependencies) for things like static analysis. that shaved off tons of time, especially for projects with lots of dependencies, some of which may not even have wheels and compiled from source at install time | 21:21 |
| sean-k-mooney | clarkb: what i i ment was pre-commit was faster but that was partly because we didnt optimize our pep8 targets to avoid installing the dep and building the package which was not required for most if not all the lininting check | 22:28 |
| sean-k-mooney | oh that was for cardoe | 22:28 |
| sean-k-mooney | so we got a speed up form pre-commit runign hacking/flake8 but i thikn if we tweaked the tox definition we coudl of some if not all fo it with tox | 22:29 |
| sean-k-mooney | JayF: in terms fo slow down i ran tox with a lock TOX_CONSTRAINTS_FILE and its still too 120 ish seconds jsut to creat the env | 22:30 |
| sean-k-mooney | i noticed that sometiem the pbr install can be pretty slow but i have not had time to debug why | 22:30 |
| clarkb | pbr is installed as a setup requires so you don't get the benefit of the pip cache iirc | 22:31 |
| clarkb | though tox doesn't use pip right? is does its own thing now? so maybe its tox specific | 22:32 |
| cardoe | no tox uses pip | 22:33 |
| sean-k-mooney | it still uses pip i belive but venvs dont containt pip by defualt anymore | 22:34 |
| sean-k-mooney | perhaps that is what your thinking of | 22:34 |
| cardoe | sean-k-mooney: I believe pbr always invokes build which creates a virtualenv cause of how tox works | 22:34 |
| clarkb | tox switched to its own pacakge building system and I thought pip would do that. but maybe it is only for the specific package under test rather than all the deps | 22:34 |
| sean-k-mooney | yes that the build isolation feature | 22:34 |
| sean-k-mooney | but that seperate | 22:34 |
| sean-k-mooney | i think its related to the resolution takign a long time but i need to look at it again with verbose loging | 22:35 |
| clarkb | oh indeed no wheel on https://pypi.org/project/pbr/#files | 22:35 |
| clarkb | I wonder why. fungi may know if that is intentional or an oversight | 22:35 |
| sean-k-mooney | just to be clear that 120+ setup pahse was on a phsical system i was runing tox on a few hours ago | 22:36 |
| clarkb | sean-k-mooney: pbr doesn't depend on anything so dep resolution should be a noop | 22:36 |
| sean-k-mooney | but i also saw simierl timing on one of my home system a week or two ago | 22:36 |
| sean-k-mooney | clarkb: ya i need to run it agent let me see if i can repoduce it quickly | 22:36 |
| sean-k-mooney | or slowly... | 22:36 |
| clarkb | it technically depends on setuptools but that is implicit (you have to list setuptools in your own pyproject.toml or preinstall it) | 22:36 |
| cardoe | no, you just put pbr in your build stuff since 6.1.1 | 22:37 |
| clarkb | you still have to list setuptools though iirc. PBR lists setuptools as a build requires but not a make a package for something else requires | 22:39 |
| clarkb | it seems to run the normal pulish-to-pypi jobs. I still don't understand why there isn't a wheel | 22:40 |
| sean-k-mooney | pbr might be a redherring it was the package that was slow liek 3 weeks ago wheni forst started seeign the slowness but that could have been a one off | 22:42 |
| clarkb | the default is to build a wheel and an sdist | 22:43 |
| clarkb | I don't understand why there is no wheel. The intent seems to be that we would build and publish a wheel, but yes not having a wheel will cause the install process to grab the sdist, build a wheel, then install the wheel | 22:45 |
| sean-k-mooney | py3: OK (359.42=setup[350.51]+cmd[8.91] seconds) | 22:48 |
| clarkb | looks like the upload step has a filter on wheel filenames to avoid uploading x86 specific wheels (it wants to upload platform indepednent wheels which I would expect the pbr wheel to be | 22:49 |
| clarkb | but maybe it isn't for some reason and that would be the problem | 22:49 |
| sean-k-mooney | i should log that to a file but it might be the requirement file but im not sure. it just seamed to hang for no apprent reasons | 22:49 |
| clarkb | sean-k-mooney: is that running without constraints? without constraints you would need to dep solve | 22:50 |
| clarkb | without a proper log its hard to say where time is going though | 22:50 |
| sean-k-mooney | no but ill do this again and log it to a file | 22:51 |
| sean-k-mooney | that happend to be just the cybrog unit test but i have seen it on nova too | 22:51 |
| sean-k-mooney | https://github.com/openstack/cyborg/blob/master/tox.ini#L5 | 22:52 |
| sean-k-mooney | we currently have 2 patrens | 22:52 |
| sean-k-mooney | most of the libs are or were set up like cybrog | 22:52 |
| sean-k-mooney | and most of the service were setup like nova https://github.com/openstack/nova/blob/master/tox.ini#L13 | 22:53 |
| clarkb | another thing that can cause slowness is needing to build a wheel that actually compiles | 22:53 |
| sean-k-mooney | sith ethe constriatn in the intsll command | 22:53 |
| clarkb | for nova the classic example is libvirt python and lxml | 22:53 |
| sean-k-mooney | ya so in the prior un i think almost every ting was forom the cache | 22:53 |
| clarkb | but I think lxml started publishing wheels eventually but libvirt python never has due to some conflict over license interpretation or something | 22:53 |
| sean-k-mooney | as i had just doing an instlal with -r before | 22:53 |
| sean-k-mooney | well not libvirt python has some dynmic code that depend on the libvirt its linked against | 22:54 |
| sean-k-mooney | i.e. it generatis constants based on the constisn defiend in the libvirt headers its built against | 22:55 |
| sean-k-mooney | so they dont publish wheels so that ti will be compatible with the libvirt on your host | 22:55 |
| sean-k-mooney | i belive they gaurentee or at least try to gaurenteee that if your libvirt-python is newer or the same version as your host version it shoudl alwasy be compatible | 22:56 |
| clarkb | I think they have backward and forward comaptibility so it is possible to determine that at runtime rather than compile time iirc | 22:56 |
| clarkb | the main issue was some pypi license thing | 22:56 |
| clarkb | https://zuul.opendev.org/t/openstack/build/4c10929ab2044928a50c635d771cbe75/log/job-output.txt#540-1601 here it says it took 83 seconds to setup the venv without any local caches | 22:58 |
| clarkb | ~6 seconds of that was handling pbr | 22:59 |
| clarkb | ~50 seconds was downloading packages from pypi | 23:00 |
| clarkb | ~19 seconds was installing the deps after the downloaded and a few wheels were built | 23:01 |
| clarkb | installing cyborg itself took about 5 seconds | 23:02 |
| clarkb | which leaves me 2 seconds unaccounted for I think | 23:02 |
| sean-k-mooney | https://pb.teim.app/?e19192d6c651557c#xxQBCdveohYbW9ABybVGSCNmaqdnuNg4bbu6iXXpkyW was my local run | 23:02 |
| sean-k-mooney | it stoped on the pipe install lines for several second in both cases | 23:02 |
| clarkb | that doesn't include any timestamps | 23:02 |
| sean-k-mooney | ya im tryign to figure out how to do that | 23:03 |
| sean-k-mooney | can tee add them? | 23:03 |
| sean-k-mooney | im just piping the output of tox to a file | 23:03 |
| clarkb | I think devstack may have a helper function thing that overrides writing to stdout | 23:04 |
| clarkb | and it prepends the times? | 23:04 |
| sean-k-mooney | i can take a look | 23:05 |
| sean-k-mooney | what i can see is the fist pip install took 55 seconds py3: 190475 I exit 0 (55.64 seconds) | 23:05 |
| sean-k-mooney | to install everyting form the local cache | 23:05 |
| sean-k-mooney | the second was 47 seocdn for the build env | 23:06 |
| sean-k-mooney | .pkg: 237929 I exit 0 (47.30 seconds) /home/debian/repos/cyborg> python -I -m pip install 'pbr>=6.0.0' 'setuptools>=64.0.0' pid=2173096 [tox/execute/api.py:311] | 23:06 |
| sean-k-mooney | ok so that interesting actully | 23:07 |
| sean-k-mooney | i mentioned htere are 2 ways we pass the constiats file | 23:08 |
| clarkb | which is a big difference to teh upstream test job | 23:08 |
| sean-k-mooney | for services we normally put it in the install command | 23:08 |
| clarkb | the download and install of the deps takes longer there but installing pbr and setuptools is much faster | 23:08 |
| sean-k-mooney | ther e is no constriat file specificed for the build deps for the buidl venv there | 23:09 |
| sean-k-mooney | i wond if we put it in the istnall command will it be there | 23:09 |
| clarkb | and your local install of pbr and setuptools says it is using cached wheels so no builds for either of them | 23:09 |
| clarkb | I half wonder if finding things in the cache is slower than fetching and building them | 23:10 |
| sean-k-mooney | that woudl be ironic but perhaps | 23:11 |
| sean-k-mooney | puting the socnteit file in the install command does nto change teh setup of the isolated build venv for what its worth | 23:11 |
| sean-k-mooney | the run was fster over all but its not deterministic which is alwasy fun | 23:12 |
| clarkb | pkg_resources was super sensitive to disk io too (still is actually). I wonder if they've introduced something like that in the candidate pacakge discovery process | 23:13 |
| clarkb | pkg_resources did a for every package you can find in python path load all of its metadata, then sort the giant list based on package name and version sort of thing | 23:13 |
| clarkb | so as the number of packages installed on a system (including old completely ignored versions) grew over time so did wall time to run every single python command | 23:14 |
| clarkb | virtualenvs mitigate against this, but the pip cache is central. I wonder if it grows to a certain size and then you're no longer saving time (this is all hand wavy based on what pkg_resources did I haev no direct evidence of that happening here) | 23:14 |
| sean-k-mooney | i could check or clear my cache and see | 23:15 |
| sean-k-mooney | i orginally though it was relate to doing `uvx tox -e ...` but i have seeing the same issue with tox form the devstack ven v and venvs i chreate manually | 23:16 |
| sean-k-mooney | at the end of the day usign pipx or uvx is realy just puting the tool in a venv you dont typielcy see in a sub dir of your home directory and that not where it hanging it alwasy in the execution of tox | 23:18 |
| sean-k-mooney | what is interesting is localy at lest /home/debian/.cache/pip/wheels/ is pretty much empty and most of the cached artifacts are under /home/debian/.cache/pip/http-v2 | 23:23 |
| sean-k-mooney | implying it was pullign the sdist by defualt | 23:24 |
| sean-k-mooney | well they could be egg files i guess they are binary | 23:25 |
| sean-k-mooney | lol pbr>=0.11,!=2.1.0 # Apache-2.0 that probably isnt helpeing | 23:28 |
| sean-k-mooney | empty cache took about the same but thats a problem for anohter day | 23:32 |
| sean-k-mooney | py3: OK (303.69=setup[294.54]+cmd[9.15] seconds) | 23:32 |
| sean-k-mooney | a cached run that does not hang is tsill a lot faster | 23:34 |
| sean-k-mooney | py3: OK (90.11=setup[80.94]+cmd[9.17] seconds) | 23:34 |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!