| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-19 | |||
| 14:03:35 | stephenfin | efried: ^ | |
| 14:05:17 | efried | ack | |
| 14:17:02 | mriedem | gibi: now that there has been more review on the cross-cell-resize series, i'm thinking about moving https://review.opendev.org/#/c/642590/ and https://review.opendev.org/#/c/642183/ and https://review.opendev.org/#/c/637605/ to the front of the series to get those merged out of the way (next time i do a rebase) - any problems with that? | |
| 14:20:10 | mriedem | dansmith: this is a pretty simple +2d bottom of the cross-cell series which is the prep_resize equivalent and would help move a few things https://review.opendev.org/#/c/633293/ - it's basically just a call to the dest compute to do a resize_claim for pci/numa and send back the migration context | |
| 14:25:29 | dansmith | mriedem: okay | |
| 14:26:52 | dansmith | mriedem: is this not something we should be doing all the time to make numa lm easier? | |
| 14:27:02 | dansmith | because they're kinda stuffing the claim into other calls right now | |
| 14:28:27 | mriedem | meaning the numa lm series is doing the move claim as part of pre_live_migrate_at_dest or whatever, right? | |
| 14:28:36 | dansmith | yeah, | |
| 14:28:37 | mriedem | and same-cell resize/cold migrate does it in prep_resize | |
| 14:29:13 | dansmith | I had just read the commit message when I said ^ but it struck an immediate chord with me | |
| 14:29:20 | dansmith | that we're going to have three ways for the three types, AFAICT | |
| 14:30:17 | mriedem | i would need to refresh on the live migration claim to know if it's needing to use migrate data from the source or anything | |
| 14:30:30 | dansmith | it does | |
| 14:30:36 | efried | aspiers: Didn't you write a mock_open wrapper? | |
| 14:30:43 | aspiers | I did | |
| 14:30:53 | mriedem | prep_resize for same-cell is quite a bit different from the cross-cell one since it's a cast, does reschedule logic, and then casts to the resize_instance method on the source | |
| 14:30:53 | aspiers | patch_open and patch_exists | |
| 14:31:04 | mriedem | so i'm not sure that trying to munge those into a single method to work for all is going to be better | |
| 14:31:11 | mriedem | feels like the rebuild/evacuate trap to me | |
| 14:31:20 | aspiers | efried: and yes, foo^! is foo^..foo | |
| 14:31:24 | sean-k-mooney | aspiers: how do you determin all test that need to be run for a change | |
| 14:31:26 | efried | aspiers: found it | |
| 14:31:44 | sean-k-mooney | reliable that is | |
| 14:31:51 | efried | sean-k-mooney: he's not mapping source changes to tests. | |
| 14:31:57 | aspiers | sean-k-mooney: I don't yet, but you would need to hardcode those mappings | |
| 14:32:01 | efried | sean-k-mooney: he's just running test files you changed in the commits | |
| 14:32:11 | aspiers | which is exactly what you can do with a Guardfile | |
| 14:32:25 | mriedem | efried: so no tests no failures yay! | |
| 14:32:31 | sean-k-mooney | ok that | |
| 14:32:51 | aspiers | hell, that's the kind of thing people used to do with Makefiles, in the olden days before actually useful dependency trees went out of fashion and all the modern bullshit came in | |
| 14:32:51 | efried | aspiers: I was thinking you could get partway by mapping nova/a/b/c.py to nova/tests/{unit|functional}/a/b/test_c.py | |
| 14:32:52 | sean-k-mooney | will likely miss relevent tests then | |
| 14:32:58 | efried | and then hardcode anything that doesn't hit with that. | |
| 14:33:01 | aspiers | efried: yes for sure | |
| 14:33:31 | mriedem | efried: note that lots of the modules aren't 1:1 like that | |
| 14:33:35 | sean-k-mooney | i really dont like the idea of hardcoding any mappings | |
| 14:33:50 | efried | mriedem: Yes, I get that, I'm saying that would get you a big chunk for free, and then you hardcode the rest. | |
| 14:33:57 | mriedem | gross | |
| 14:34:00 | efried | but that's not the point of this patch at all. | |
| 14:34:13 | efried | this patch just runs "tests I changed in XXX commit(s)" | |
| 14:34:13 | mriedem | it's just auto-run the test module changes in my commit, i know | |
| 14:34:29 | aspiers | sean-k-mooney: the entire CI test suite is "relevant" in a sense. The point of locally rapidly running a small subset of tests every time you make a local code change is not to be complete, it's to catch the 80% of stupid mistakes and make sure the tests you *are* changing on are correct | |
| 14:34:49 | mriedem | this is proposed as totally optional yes? | |
| 14:34:53 | mriedem | meaning i never have to deal with this? | |
| 14:34:58 | aspiers | yes | |
| 14:35:10 | sean-k-mooney | ya i get that but generally i jsut use the regex support for that | |
| 14:35:20 | mriedem | same, | |
| 14:35:26 | mriedem | tox -e py36 -- test_compute | |
| 14:35:29 | aspiers | sean-k-mooney: but then you have to maintain lists of regexes. the point is to save you the hassle for quick obvious stuff | |
| 14:35:35 | sean-k-mooney | e.g. tox -e py36 -- "libvirt|compute" | |
| 14:35:58 | sean-k-mooney | aspiers: no you just add words that relate to the thing your doing when you type it | |
| 14:36:00 | aspiers | right, except that will take like 20-30s longer because a) it goes through tox and b) it does test discovery | |
| 14:36:24 | aspiers | sean-k-mooney: sure but that's a different use case | |
| 14:36:24 | sean-k-mooney | if i change ovo i add objecst if i modify the libvirt dirver i add libvirt | |
| 14:36:48 | aspiers | I'm not proposing to replace any existing workflows, just add the option of a quick and easy extra one | |
| 14:36:55 | sean-k-mooney | right but if i want to run just one test it add teh test name or use an ide. | |
| 14:37:05 | aspiers | it's already saved me a bunch of typing this morning since I wrote it | |
| 14:37:19 | sean-k-mooney | anyway it nice to have a script for the new stuff | |
| 14:37:33 | sean-k-mooney | just i dont want it to imply that it will run all the relevent tests | |
| 14:39:46 | sean-k-mooney | aspiers: its good for saving time locally as long as it does not encurage people to waste gate time but not running the test they should have locally | |
| 14:40:00 | openstackgerrit | Adam Spiers proposed openstack/nova master: Switch some GitHub URLs to point to opendev.org https://review.opendev.org/677185 | |
| 14:40:37 | aspiers | sean-k-mooney: I'm not sure how providing another way to run tests locally would discourage people from running tests locally ;-) | |
| 14:41:02 | sean-k-mooney | aspiers: if that is all they run they will miss test they should have run | |
| 14:41:17 | aspiers | tools are never gonna be able to stop developers from being stupid | |
| 14:41:41 | sean-k-mooney | yes but it can make it easier for them to be :) | |
| 14:41:48 | aspiers | sure but at some point you just have to trust people to use their brain | |
| 14:41:54 | aspiers | and this is not rocket science | |
| 14:42:10 | aspiers | noone is saying "OK now we have this new tool and that is the ONLY tool you should eer use" | |
| 14:42:14 | aspiers | ever | |
| 14:42:57 | sean-k-mooney | i know but even with the regex approch its rare but sometime if i dont do a full tox run i will miss a test that the gate finds | |
| 14:43:06 | aspiers | I'd also argue that developer time is more expensive than gate time, but that's a whole other debate | |
| 14:43:15 | openstackgerrit | Eric Fried proposed openstack/nova master: Tests: autospecs all the mock.patch usages https://review.opendev.org/470775 | |
| 14:43:21 | aspiers | yeah, so use your regex approach | |
| 14:43:43 | aspiers | or, use my new approach while you are in the middle of coding because it's quicker, then use yours when you think you are done and ready to upload | |
| 14:43:56 | aspiers | doing both gets the best of both worlds | |
| 14:44:17 | aspiers | this is just basic 80/20 rule application | |
| 14:44:36 | efried | aspiers: this bud's for you https://review.opendev.org/#/c/470775/42/nova/tests/unit/test_json_ref.py@58 | |
| 14:45:57 | sean-k-mooney | ya i get that. i like fast8 for example but that is always correct. | |
| 14:46:02 | aspiers | efried: you mean you want me to submit a separate fixup later? | |
| 14:46:20 | aspiers | sean-k-mooney: running a single test is always correct, it's not just complete :) | |
| 14:46:24 | aspiers | this is no different to fast8 | |
| 14:46:30 | aspiers | fast8 is not complete either | |
| 14:46:31 | efried | aspiers: "want"... it's a suggestion I thought might interest you :) | |
| 14:46:37 | mriedem | sean-k-mooney: a few more small things in the pmu change https://review.opendev.org/#/c/671338/11 | |
| 14:46:48 | aspiers | efried: indeed, it will make me happy to see others using it :) | |
| 14:47:32 | sean-k-mooney | mriedem: ya i just got the email for your ps8 comments | |
| 14:47:36 | sean-k-mooney | ill look at them now | |
| 14:48:16 | sean-k-mooney | are you ok with me moveing the bool_form_strng as i have in the latest version? | |
| 14:49:21 | sean-k-mooney | assuming i add the new tests you suggested | |
| 14:50:35 | efried | mriedem: I think you were complaining about how long it takes to build specs at some point, and since you're clearly so enthusiastic about this kind of workflow improvement: https://review.opendev.org/#/c/673356/ | |
| 14:51:18 | mriedem | complain yes, but not so much to want to review a bash script :) | |
| 14:52:19 | sean-k-mooney | efried: the specs are not too bad. recently started building releasenotes locally for some of my patches and was suprised at how long those take. | |
| 14:52:21 | dansmith | mriedem: I dropped some comments on the bottom two patches, but I'm still mulling my feelings about it, but am out of time before my next call | |
| 14:53:07 | dansmith | mriedem: it really seems like an unfortunate purpose-built call to do part of the work of the other two for a specific situation, which just feels wrong and very fragile | |
| 14:53:09 | dansmith | I know it's more work to refactor, and I don't really want to review *that* either | |
| 14:53:16 | dansmith | so I dunno | |
| 14:57:15 | mriedem | to refactor prep_resize? | |
| 14:57:58 | dansmith | well, all of them to do this as a single thing | |
| 14:58:12 | dansmith | I dunno, I'll try to think on it when I'm done with my next call | |