| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-27 | |||
| 16:28:48 | openstackgerrit | sean mooney proposed openstack/os-vif master: add addtion check and gate jobs for os-vif https://review.openstack.org/639732 | |
| 16:28:50 | openstackgerrit | Jim Rollenhagen proposed openstack/nova master: ironic: check fresh data when sync_power_state doesn't line up https://review.openstack.org/636699 | |
| 16:31:37 | mriedem | dcdawg: definitely don't need a bug | |
| 16:32:09 | mriedem | depending on the comment you're fixing, if the comment is otherwise not something that can be understood or make sense and needs to be re-written for clarity, sure that's fine | |
| 16:32:30 | mriedem | it's just a simple typo but you still understand what the comment is saying, then patches to fix those are mostly just noise, unless you fix a bunch of stuff in a single change | |
| 16:34:00 | efried | Hi nova. Do the current args passed into a filter (which appear to be a HostState and a RequestSpec) somewhere contain information from GET /a_c? Namely | |
| 16:34:00 | efried | a) the allocation candidate (allocation_requests[N]) under consideration; and | |
| 16:34:00 | efried | b) (the relevant subset of) the provider_summaries? | |
| 16:35:09 | mriedem | pretty sure no | |
| 16:36:02 | efried | HostState.host is a Selection object? | |
| 16:36:10 | mriedem | no, | |
| 16:36:13 | mriedem | it's a ComputeNode.host | |
| 16:36:27 | mriedem | HostState is a wrapper of a ComputeNode | |
| 16:36:47 | gibi | mriedem, stephenfin: thanks for the reviews | |
| 16:37:13 | efried | okay; seems like a and b should be something we should start passing into filters, nah? | |
| 16:37:34 | mriedem | we haven't had a need yet | |
| 16:37:40 | efried | as soon as nested starts being real in allocation candidates... | |
| 16:37:44 | mriedem | when you have a filter that needs that information then we'll talk about it | |
| 16:37:53 | efried | and until we have every possible filtering and weighing feature natively in placement... | |
| 16:38:04 | mriedem | which we won't | |
| 16:38:11 | efried | right | |
| 16:39:05 | efried | okay, that's fair. But it seems like it would be pretty easy to do. | |
| 16:39:48 | mriedem | nothing is ever easy anymore | |
| 16:39:54 | cdent | was it ever? | |
| 16:39:59 | mriedem | sure | |
| 16:40:07 | mriedem | back in grizzly you could do whatever :) | |
| 16:40:15 | mriedem | it didn't need to actually work | |
| 16:40:41 | mriedem | https://media2.giphy.com/media/11fpbdcbnBd5tu/source.gif | |
| 16:41:16 | cdent | efried: you have a concrete use case yet, or preparing for the future? | |
| 16:42:06 | efried | cdent: more the latter. | |
| 16:42:40 | cdent | the situation seems to be "lots of things in nova are going to want that info, eventually" | |
| 16:42:49 | efried | I agree with that. | |
| 16:43:10 | efried | But I also sympathize with the notion of: let's not do this just because we think it's a good idea; let's wait until someone actually needs it. | |
| 16:43:11 | cdent | It's the eventually that seems to be causing strain | |
| 16:43:17 | efried | YAGNI? | |
| 16:43:30 | cdent | there's a growing movement to make sure we have contributors who will own that stuff | |
| 16:44:18 | cdent | I guess yagni's a factor, sure, but more: let's not add extra work without adding extra people. If people don't show up, they must not want it. | |
| 16:44:35 | cdent | making that void more visible is a thing I care about quite a bit | |
| 16:44:47 | mriedem | artom: ^ sound familiar? | |
| 16:45:04 | artom | mriedem, heh, I was gonna say | |
| 16:45:10 | jaypipes | jroll: sorry, yes, please go ahead | |
| 16:45:18 | cdent | mriedem: ? | |
| 16:45:45 | mriedem | cdent: artom and i were just talking about that, in a closet | |
| 16:45:57 | artom | Yeah, we've come out of the closet just now | |
| 16:46:05 | cdent | congrats | |
| 16:47:20 | cdent | so how to visibilate things/ | |
| 16:47:40 | cdent | (not including hickies) | |
| 16:47:48 | edleafe | This should be added somewhere in the OpenStack charter: "let's not | |
| 16:47:48 | edleafe | add extra work without adding extra people. If people don't show up, | |
| 16:47:51 | edleafe | they must not want it." | |
| 16:48:10 | artom | edleafe, people *do* show up. Once. | |
| 16:48:33 | artom | And as mriedem was saying, in 2 years of deployers upgrade to the new thing, those people are gone. | |
| 16:48:38 | artom | *when | |
| 16:48:42 | mriedem | i'm not in my 20s anymore, i'm looking for a long-term commitment with contributors | |
| 16:50:18 | artom | o/` If you want it better put a ring on it o/` | |
| 16:56:37 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP status: Ensure templated connection URLs are rendered before use https://review.openstack.org/639607 | |
| 17:03:54 | efried | mriedem_away, jaypipes, bauzas, gibi: I think https://review.openstack.org/#/c/538498/ is ready to go now, if you want to give it another look-see. | |
| 17:04:02 | efried | (convert compute caps to traits) | |
| 17:08:44 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add remove_resources_from_instance_allocation to report client https://review.openstack.org/639653 | |
| 17:08:44 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Remove port allocation during detach https://review.openstack.org/622421 | |
| 17:08:45 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Record requester in the InstancePCIRequest https://review.openstack.org/625310 | |
| 17:08:45 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add pf_interface_name tag to passthrough_whitelist https://review.openstack.org/625311 | |
| 17:08:46 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Ensure that bandwidth and VF are from the same PF https://review.openstack.org/623543 | |
| 17:08:46 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support server create with ports having resource request https://review.openstack.org/636360 | |
| 17:08:47 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Refactor _heal_allocations_for_instance to make place for port healing https://review.openstack.org/637953 | |
| 17:08:47 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Refactor _heal_allocations_for_instance (2) https://review.openstack.org/637954 | |
| 17:08:48 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: nova-manage: heal port allocations https://review.openstack.org/637955 | |
| 17:08:48 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: cache neutron ports in heal allocation https://review.openstack.org/638207 | |
| 17:09:52 | gibi | mriedem_away: I fixed up https://review.openstack.org/#/c/639653 but I run out of time on https://review.openstack.org/#/c/622421 so I have to do that tomorrow | |
| 17:13:07 | openstackgerrit | Merged openstack/nova master: Fup for the bandwidth series https://review.openstack.org/639159 | |
| 17:15:25 | jaypipes | efried: still says cannot merge on that patch... | |
| 17:15:30 | jroll | jaypipes: oops, I meant I left a question in gerrit, sorry | |
| 17:15:40 | efried | oh? | |
| 17:15:56 | efried | oh. | |
| 17:16:20 | efried | aspiers: ^ looks like needs rebase. Which ugh probably means need to wait until predecessor finishes merging. | |
| 17:18:00 | melwitt | does anyone know what's the correct way to unit test code that calls placement? I found the PlacementFixture nova/tests/functional/fixtures.py but was thinking that's not appropriate for unit tests | |
| 17:22:33 | efried | melwitt: "unit" and "calls placement" don't really gel. If it's a true unit test, the placement calls ought to be mocked. | |
| 17:23:31 | melwitt | efried: right.. I wondered if there was a specific way to mock them (like a fixture I don't know about) or if it's just ad-hoc | |
| 17:23:48 | efried | melwitt: Do you have a specific example? | |
| 17:23:50 | melwitt | I didn't find any example yet. still looking | |
| 17:23:53 | efried | okay. | |
| 17:24:11 | melwitt | I'm working on quota check code that calls placement, so once I added the placement call, lots of unit tests fail | |
| 17:24:29 | melwitt | and if someone happened to know the recommended way to mock placement, that would help me | |
| 17:24:48 | efried | melwitt: See nova.tests.unit.scheduler.client.test_report | |
| 17:25:17 | efried | melwitt: Those tests mock SchedulerReportClient.<http primitive like get/put/post/delete> | |
| 17:25:18 | melwitt | thank you | |
| 17:26:24 | efried | melwitt: Whereas nova.tests.functional.test_report_client uses the placement fixture to create an interceptor context manager with which to wrap "real" calls to placement. | |
| 17:27:01 | efried | the latter, for functional tests, gets you real placement behavior. The former just lets you pretend you got a certain Response object. | |
| 17:27:17 | melwitt | efried: got it. thanks for the help | |
| 17:27:23 | efried | np, enjoy. | |
| 17:31:27 | efried | melwitt: correction: test_report_client uses PlacementDirect. The difference in usage between that and PlacementFixture is the context manager-ness. | |
| 17:31:55 | efried | but it sounds like you want the unit level stuff in any case. | |
| 17:33:24 | melwitt | right. but that's good info to know anyway. I'll need to use that too for the functional tests I think | |
| 17:34:10 | openstackgerrit | sean mooney proposed openstack/os-vif master: add addtion check and gate jobs for os-vif https://review.openstack.org/639732 | |
| 17:34:32 | efried | melwitt: In case I don't otherwise notice, lmk when you've got code up, would like to review. | |
| 17:35:47 | melwitt | efried: the patch that adds placement calls is here https://review.openstack.org/638073 implementation done, test coverage is what I'm working on now | |
| 17:36:33 | efried | ack | |
| 17:45:32 | efried | melwitt: left a comment. So yeah, if you take that advice, you'll probably want a functional test in test_report_client; and you may decide a unit test isn't necessary. | |
| 17:46:30 | melwitt | efried: sweet, thanks | |
| 17:46:57 | efried | yw | |