| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-19 | |||
| 13:09:46 | stephenfin | efried: I've noticed that there are many more gaps in test coverage with that privsep series. Are you okay with a giant follow-up to close said gaps? | |
| 13:09:57 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420 | |
| 13:10:14 | stephenfin | Cos I'll have to recuse myself from reviews if I go add said missing tests to each patch | |
| 13:10:38 | stephenfin | unless you're happy to single +2 the tests, that is | |
| 13:10:43 | efried | stephenfin: I have no doubt there are gaps in test coverage overall, but what bugs me is the coverage being reduced by each patch. Is that what you're talking about? | |
| 13:10:44 | aspiers | stephenfin: responses to your feedback there ^^^ | |
| 13:10:56 | stephenfin | efried: Yeah, that specifically | |
| 13:11:11 | stephenfin | i.e. missing tests in nova/tests/unit/privsep | |
| 13:11:13 | stephenfin | aspiers: sweet | |
| 13:11:17 | efried | In that case, yes, a fup is good for me, but I'll want to wait to merge a given patch until its coverage is at least proposed. | |
| 13:11:50 | aspiers | stephenfin: but you might find more enjoyment in https://review.opendev.org/#/c/677180/ | |
| 13:12:00 | efried | and yes, good plan wrt reviews, since it's tough to convince other cores to care about that series, we should take advantage of the two of us being willing to dig into it :P | |
| 13:12:01 | aspiers | efried: you too :) | |
| 13:12:41 | aspiers | my dream is never having to *manually* run tests locally again | |
| 13:12:52 | aspiers | just edit files and watch things turn red or green | |
| 13:13:07 | aspiers | it's entirely doable | |
| 13:13:14 | aspiers | I've done it with many other projects | |
| 13:13:17 | stephenfin | efried: Yeah, of course. Maybe just keep an eye out for gaps as you're reviewing so I've something to hit "Done" on | |
| 13:14:25 | efried | wfm | |
| 13:15:59 | efried | aspiers, stephenfin: Love this idea, add it to the list of workflow improvements it's hard to get people to pay attention to: https://review.opendev.org/#/c/673356/ | |
| 13:16:29 | aspiers | efried: personally I *never* tire of workflow improvements | |
| 13:16:39 | aspiers | you can always count me in for reviewing those | |
| 13:18:04 | efried | aspiers: what's mybranch^! ? | |
| 13:18:21 | aspiers | the commit at the tip of mybranch | |
| 13:18:31 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: Exclude build request marker from server listing https://review.opendev.org/677201 | |
| 13:19:02 | aspiers | efried: https://git-scm.com/docs/git-rev-parse#_other_rev_parent_shorthand_notations | |
| 13:19:21 | stephenfin | Well it's not a workflow improvement, but this would save us some time https://review.opendev.org/#/c/470775/ | |
| 13:19:52 | stephenfin | (make mock fail if you call a mocked function incorrectly or a function that doesn't exist) | |
| 13:20:35 | stephenfin | and it's been around in one form or another for...2 years, 2 months and 17 days :'D | |
| 13:21:05 | efried | yeah, I thought claudiub had been working on this for ages and was never able to get it quiiite right. | |
| 13:21:09 | efried | is it right now? | |
| 13:21:19 | efried | I guess you think so according to your +2 :) | |
| 13:21:48 | stephenfin | It all looks sane and I'm relying on CI to catch anything we might have missed | |
| 13:21:55 | stephenfin | so...yes. | |
| 13:22:47 | efried | aspiers: So foo^! is like foo~1..foo ? | |
| 13:24:59 | efried | I admit, I learned like two of the weird shorthands and then cordoned off the rest of my brain for other things. | |
| 13:25:13 | efried | 80/20 rule | |
| 13:27:01 | sean-k-mooney | woudl foo~1..foo not be git show | |
| 13:27:16 | sean-k-mooney | e.g. compare ing n-1 to n | |
| 13:27:41 | sean-k-mooney | which is the diff of n or what git show displays | |
| 13:27:48 | efried | if you're using it for a diff, yes | |
| 13:27:55 | sean-k-mooney | ok | |
| 13:28:01 | sean-k-mooney | just makeing sure | |
| 13:28:55 | sean-k-mooney | the ~1 thing always confused me | |
| 13:29:15 | sean-k-mooney | i only ever use it if im doing "git reset --hard HEAD~1" | |
| 13:29:37 | sean-k-mooney | to remove the top commit form my branch when i commit to master by mistake | |
| 13:33:34 | efried | git checkout -b master origin/master | |
| 13:33:34 | efried | git branch -m oops | |
| 13:33:34 | efried | when I commit to master by mistake, I do something like: | |
| 13:35:54 | stephenfin | Somewhat related https://github.blog/2019-08-16-highlights-from-git-2-23/ | |
| 13:36:40 | stephenfin | (git 2.23 adds experimental 'git restore' and 'git switch' commands that each do a bit of what 'git checkout' is doing currently) | |
| 13:47:03 | mriedem | gibi: answered your question about multi-cell migration reschedule https://review.opendev.org/#/c/635668/41 | |
| 13:47:23 | mriedem | gibi: tl;dr yes it's a todo, i just didn't do it yet since i wanted to get the main flow written, tested and reviewed first | |
| 13:47:23 | yonglihe | stephenfin: Thanks, its stressful. | |
| 13:52:45 | gregwork | what kind of access does a tenant need to create private flavors within their tenant | |
| 13:54:50 | gregwork | getting ERROR: You are not authorized to use resource_types: OS::Nova::Flavor when deploying a stack in this tenant as _member_... I also get this error as ResellerAdmin and Creator | |
| 13:55:30 | gregwork | the "admin" role appears to transform the tenant into a cloud operator able to see and modify all other tenants so this is not useful | |
| 14:03:31 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Address nits from privsep series https://review.opendev.org/677210 | |
| 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 | |