| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-21 | |||
| 16:21:05 | mriedem | efried_afk: did cern reach out to you about the deepcopy of the ProviderTree? i think it came up last week, but was reminded after reading https://techblog.web.cern.ch/techblog/post/nova-ironic-at-scale/ | |
| 16:21:56 | openstackgerrit | Matthew Booth proposed openstack/nova master: libvirt: Fix service-wide pauses caused by un-proxied libvirt calls https://review.opendev.org/677736 | |
| 16:22:37 | mriedem | aspiers: you mean moving driver.py tests out of test_driver.py? but leave the code in driver.py? that would be confusing to me, and becomes a mess for git history tracking | |
| 16:23:25 | mriedem | there is probably a ton of copy/paste patterns in libvirt driver tests that could be a simple cleanup to start with, rather than moving things *out* of the test_driver.py module | |
| 17:02:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Get instances outside of update_available_resource lock https://review.opendev.org/677790 | |
| 17:29:59 | openstackgerrit | Merged openstack/nova master: Move calls to ovs-vsctl to privsep. https://review.opendev.org/639282 | |
| 17:32:01 | openstackgerrit | Merged openstack/nova master: Move setting of device trust to privsep. https://review.opendev.org/639283 | |
| 17:44:58 | efried_afk | aspiers: I love the idea of splitting huge files. sean-k-mooney, syntax highlighting isn't the problem so much as the inspection. Every change causes it to reinspect the file, which takes forever on monsters like test_libvirt. If I'm spending any amount of time there, I have to turn inspection off. | |
| 17:45:50 | dansmith | efried_afk: without any other context or target articles, | |
| 17:46:01 | dansmith | that first sentence sounds like it belongs on your online dating profile | |
| 17:46:10 | dansmith | "I like long walks on the beach and splitting huge files" | |
| 17:47:03 | sean-k-mooney | efried_afk: i have not had that problem but it does annoy me that thost files are so large | |
| 17:47:06 | efried | dansmith: It was a straight copy/paste | |
| 17:47:14 | dansmith | efried: I figured | |
| 17:47:46 | dansmith | Male seeking cybernetic female analog capable of introspection of long python files and/or automated splitting thereof | |
| 17:47:52 | efried | mriedem: I don't recall anything specific about a ProviderTree deepcopy... | |
| 17:48:25 | efried | oo, that's good dansmith. Clearly you have a lot of experience here. | |
| 17:48:38 | dansmith | my kink knows no bounds | |
| 17:51:23 | efried | dansmith: are you happy with https://review.opendev.org/#/c/671072/ series at this point? | |
| 17:52:11 | efried | gibi: also ^ | |
| 17:52:15 | dansmith | efried: I haven't circled back | |
| 17:56:00 | openstackgerrit | Eric Fried proposed openstack/nova master: Remove deprecated [neutron]/url option https://review.opendev.org/677755 | |
| 17:56:16 | aspiers | mriedem: I would happily move the corresponding code out from driver.py at the same time if that was feasible | |
| 17:57:03 | aspiers | I'm not sure why it would become a mess for git history tracking? even git blame has for a long time been able to figure out when stuff gets moved between files | |
| 17:57:17 | aspiers | efried: exactly | |
| 17:57:47 | mriedem | i'm not a fan of the tests for something in one module living in another module | |
| 17:57:48 | aspiers | mriedem: that said, pointers on stuff to deduplicate are welcome | |
| 17:58:00 | mriedem | and like i said, i think you can just refactor the duplication in test_driver.py for lots of tests to probably make it smaller | |
| 17:58:34 | aspiers | I think our messages crossed :) ^^^ | |
| 17:58:43 | mriedem | this isn't an exercise i'm the one that's wanting to take on, so that's up to you, | |
| 17:58:55 | mriedem | but i know there is a ton of just copy test foo and tweak 1-2 lines for test bar | |
| 17:59:12 | mriedem | i also have no problem with the thing in pycharm | |
| 18:00:00 | efried | aspiers: git history is an issue, unfortunately - afaik blame will show the new commit on the new file. You would have to back up to see the previous iteration, which is awkward. (I've run into this a number of times already in placement, and it's pretty debilitating.) | |
| 18:00:25 | mriedem | correct, it sucks | |
| 18:00:36 | mriedem | the mox removal series is a prime example | |
| 18:00:45 | mriedem | it also causes issues with merge conflicts for backports | |
| 18:00:53 | sean-k-mooney | well perhaps this is jsut somethin we need to be aware of and think should we be moving things out of the driver into modules used by the driver in the future | |
| 18:01:04 | aspiers | efried: no, blame really can show where it came from | |
| 18:01:25 | sean-k-mooney | aspiers: yes but git blame on github crashes on those files | |
| 18:01:29 | sean-k-mooney | so you have to do it locally | |
| 18:01:43 | aspiers | ? | |
| 18:01:49 | efried | what's the blame trick, without iterating backward across commits? | |
| 18:01:58 | aspiers | mriedem's point about backports is a much more convincing argument against | |
| 18:02:19 | aspiers | however, if our policy is "you can't move stuff around, ever" then we're in for a miserable future | |
| 18:02:50 | mriedem | i'm not against refactoring giant methods in the libvirt driver module, deduplicating test code in test_driver, etc. i just don't want to see random tests moving out of test_driver into some other module when the code being tested is still in driver.py | |
| 18:03:20 | aspiers | mriedem: sure, that's why I said <aspiers> mriedem: I would happily move the corresponding code out from driver.py at the same time if that was feasible | |
| 18:03:21 | mriedem | aspiers: no one said that was the policy | |
| 18:03:24 | sean-k-mooney | aspiers: try opening https://github.com/openstack/nova/blame/master/nova/virt/libvirt/driver.py | |
| 18:03:36 | aspiers | mriedem: ok good :) | |
| 18:03:50 | aspiers | was beginning to sound that way | |
| 18:04:19 | mriedem | as a nova troll i have a reputation to maintain | |
| 18:04:26 | aspiers | :) | |
| 18:04:27 | mriedem | as immovable object | |
| 18:04:28 | sean-k-mooney | i think we just dont want test to be move with out the code also moving and am not sure that patches that only move code are a good idea | |
| 18:04:37 | aspiers | efried: try git gui blame for instance | |
| 18:04:55 | aspiers | some tooltips include "Copied Or Moved Here By:" | |
| 18:05:12 | aspiers | can't remember if github or gitea do the same | |
| 18:05:31 | sean-k-mooney | aspiers: i think i makes sense to consider moving things if you are refacoritn the code already. | |
| 18:05:32 | aspiers | but local is way faster anyway IME | |
| 18:05:39 | aspiers | sean-k-mooney: definitely agree with that | |
| 18:06:02 | aspiers | well, it's an idea to bear in mind anyway | |
| 18:06:28 | aspiers | 23k LoC in one file does not seem sane to me ;-) | |
| 18:07:14 | sean-k-mooney | 23K of code to test 9.75K of code and we still miss things... | |
| 18:07:43 | sean-k-mooney | the driver is not that bad coverage wise but ther are other part that are less awsome... | |
| 18:08:17 | efried | aspiers: git gui appears to be... a GUI :( :( :( | |
| 18:08:30 | aspiers | efried: shocker! who'd have thought? | |
| 18:08:33 | efried | guess it would take some getting used to. | |
| 18:09:01 | sean-k-mooney | aspiers: that is not a standard git command is it a python extentions | |
| 18:09:10 | efried | anyway, I agree enormous files suck, and being stuck with them because of sucky reasons also sucks. | |
| 18:09:19 | efried | sean-k-mooney: apt install git-gui | |
| 18:09:40 | efried | that's if you're running ubuntu. | |
| 18:09:47 | efried | If you're running another distro... just shoot yourself and be done. | |
| 18:10:07 | sean-k-mooney | PopOS which is based on ubuntu yes | |
| 18:10:18 | aspiers | efried: watch your mouth! | |
| 18:10:19 | efried | (sorry, I had a bad experience with rhel a decade ago and still haven't recovered) | |
| 18:10:31 | aspiers | you ran RHEL as a desktop? eww | |
| 18:10:34 | sean-k-mooney | although i have a rhel8 system for the last while... not sure i will keep it but it currently works | |
| 18:10:34 | aspiers | Fedora is OK | |
| 18:10:37 | efried | yeah, that was my problem | |
| 18:10:43 | aspiers | try openSUSE :) | |
| 18:10:50 | efried | is that still a thing?? | |
| 18:10:59 | efried | Is it written in FORTRAN? | |
| 18:11:09 | sean-k-mooney | aspiers: i used tumbelweed for a while and mostly liked it | |
| 18:11:13 | aspiers | ok, good time to leave | |
| 18:11:32 | sean-k-mooney | not sure i liked yast but it was nice to have up to date packages | |
| 18:49:40 | mriedem | melwitt: a couple of things in the archive patch https://review.opendev.org/#/c/507486/ | |
| 18:49:46 | mriedem | and a couple of nits in the nova-next patch on top of that | |
| 18:50:06 | mriedem | both should be small | |
| 18:50:52 | mriedem | melwitt: efried: i think we're ok to merge https://review.opendev.org/#/c/675218/6 now yeah? | |
| 18:51:11 | melwitt | thanks | |
| 18:51:49 | melwitt | yeah I think all is well | |
| 18:55:40 | mriedem | onto openstack resource provider inventory set --aggregate | |
| 18:57:35 | melwitt | heh | |
| 19:10:05 | melwitt | I was panicking with the potential loss of my launchpad bug text after a 503 "TimeoutError" but found when I reloaded the page, it posted everything I had written | |
| 19:10:16 | melwitt | phhheeewwww | |
| 19:13:21 | mriedem | happens a lot, just have to keep refreshing | |
| 19:17:34 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420 | |
| 19:21:23 | openstackgerrit | Adam Spiers proposed openstack/nova master: Extract SEV-specific bits on host detection https://review.opendev.org/636334 | |
| 19:21:34 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add |
|
| 19:22:51 | mriedem | melwitt: i think i have an idea for how to make one of multiple rps fail to set inventory in aggregate in https://review.opendev.org/#/c/640898/11 | |