| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-17 | |||
| 20:23:42 | mriedem | dansmith: so {uuid: objects.Instance(cctxt, uuid=uuid)}? | |
| 20:23:49 | dansmith | mriedem: yup | |
| 20:23:51 | mriedem | i fell it real good | |
| 20:24:10 | dansmith | mriedem: then you can just not do the conf option, it'll be slow if they touch it, and we can remove it later | |
| 20:24:11 | mriedem | dansmith: yeah that's agood point | |
| 20:24:19 | mriedem | damn good point | |
| 20:24:53 | dansmith | mriedem: snarky comments still required though | |
| 20:25:05 | dansmith | # NOTE(mriedem): Set the timer on this bomb and walk out casually | |
| 20:31:35 | mriedem | dansmith: setting the context on the instance object won't lazy-load all fields | |
| 20:31:40 | mriedem | just those in INSTANCE_OPTIONAL_ATTRS | |
| 20:31:54 | dansmith | should call _load_generic, which will do anything | |
| 20:32:03 | mriedem | if attrname not in INSTANCE_OPTIONAL_ATTRS: | |
| 20:32:08 | mriedem | raise exception.ObjectActionError( | |
| 20:32:17 | dansmith | oh, heh, | |
| 20:32:31 | dansmith | I dunno why we did that | |
| 20:32:33 | mriedem | so now my filter on instance.default_ephemeral_device no longer works | |
| 20:32:34 | dansmith | because load_generic will do it | |
| 20:33:30 | dansmith | it's from 2013, nuke it | |
| 20:34:33 | dansmith | it looks like that was just trying to make it not do an empty load for something it wasn't going to be able to satisfy, | |
| 20:34:44 | dansmith | since it was trying to figure out what to put in extra_attrs | |
| 20:35:36 | dansmith | and that was before we had load_generic | |
| 20:35:52 | dansmith | so when we added the generic thing we probably just didn't remove the thing at the top to let you use it for anything | |
| 20:36:05 | dansmith | we should also have that fill in any field it gets that isn't already set, | |
| 20:36:15 | dansmith | so you don't load the instance twice for two regular fields | |
| 20:36:20 | dansmith | I can make those changes in one patch if you want | |
| 20:36:40 | mriedem | "we should also have that fill in any field it gets that isn't already set," - i believe it will do that for all non-joined fields anyway | |
| 20:36:49 | mriedem | because it just does a normal get and load from db | |
| 20:36:56 | dansmith | load_generic won't, | |
| 20:37:02 | dansmith | it will only copy over the one thing you asked for | |
| 20:37:29 | mriedem | self[attrname] = instance[attrname] | |
| 20:37:29 | mriedem | if instance.obj_attr_is_set(attrname): | |
| 20:37:29 | mriedem | oh i see | |
| 20:37:33 | dansmith | yeah | |
| 20:37:33 | openstackgerrit | Merged openstack/nova master: Cleanup ugly stub in TestLocalDeleteAllocations https://review.openstack.org/561997 | |
| 20:37:35 | dansmith | I mean, | |
| 20:37:41 | dansmith | it nicely punishes them hard, | |
| 20:37:45 | dansmith | but probably not the best thing to do :P | |
| 20:38:03 | mriedem | i just sent to the ML btw | |
| 20:38:34 | mriedem | efried: http://lists.openstack.org/pipermail/openstack-dev/2018-May/130624.html | |
| 20:38:47 | efried | mriedem: ack | |
| 20:39:25 | mriedem | also related, we probably no longer need to default join on security_groups since that would only ever be populated for nova-network | |
| 20:40:43 | mriedem | dansmith: if you want, what i do need right now is to handle the NOTE in InstanceList.v | |
| 20:40:44 | mriedem | *get_uuids_by_host | |
| 20:41:28 | dansmith | hah | |
| 20:41:33 | dansmith | yeah I can do that too | |
| 20:41:54 | dansmith | there has to be some animated gif that applies well to that | |
| 20:42:15 | mriedem | i've been meaning to do that since https://review.openstack.org/#/c/563234/1/nova/api/openstack/compute/services.py@222 | |
| 20:42:42 | dansmith | ooh, yessss, I made unit tests fail with infinite recursion | |
| 20:47:10 | dansmith | heh, instance will trigger a double lazy load on itself apparently, which I uncovered by removing this check | |
| 20:47:13 | dansmith | that's nice | |
| 20:48:02 | mriedem | you also have a note in that exact code | |
| 20:48:05 | mriedem | about recursion | |
| 20:48:20 | edmondsw | any stable cores around to review https://review.openstack.org/#/c/567599 ? | |
| 20:48:33 | dansmith | I know | |
| 20:49:11 | edmondsw | been out there a while, and it's an easy review | |
| 20:56:26 | openstackgerrit | Merged openstack/nova master: Remove mox in test_xenapi.py (3) https://review.openstack.org/564645 | |
| 21:11:53 | dansmith | mriedem: funny thing happened on the way to the forum: https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2596-L2607 | |
| 21:12:40 | mriedem | edmondsw: easy for me to -1 | |
| 21:14:06 | mriedem | ha | |
| 21:14:13 | edmondsw | esberglu ^ | |
| 21:14:27 | mriedem | dansmith: one difference with that, | |
| 21:14:33 | mriedem | the project_only=False kwarg to model_query | |
| 21:14:42 | dansmith | aww | |
| 21:14:46 | mriedem | yup | |
| 21:15:17 | dansmith | well, I'll show you | |
| 21:16:29 | dansmith | er, | |
| 21:16:36 | dansmith | project_only=False by default on mode_query | |
| 21:17:24 | edmondsw | mriedem this fixes a PowerVM CI breakage. If we regressed, the PowerVM CI would blow up. And the regression would have to be a change in powervm code, so reviewers better be checking the powervm ci. Is that not sufficient? | |
| 21:17:33 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Trim the fat on HostState.instances https://review.openstack.org/569247 | |
| 21:18:05 | mriedem | damn you smith | |
| 21:20:00 | mriedem | edmondsw: it'll be a cold day in hell before i... | |
| 21:20:04 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: update add-consumer-generation to focus on API https://review.openstack.org/565565 | |
| 21:20:07 | efried | mriedem: ^ | |
| 21:21:21 | efried | and dansmith ^ | |
| 21:21:28 | openstackgerrit | Merged openstack/nova master: Remove unnecessary 'to_primitive' call https://review.openstack.org/568532 | |
| 21:21:53 | openstackgerrit | Merged openstack/nova master: Fakelibvirt migrateToURI3 should provide args according to libvirt doc https://review.openstack.org/569047 | |
| 21:27:23 | mriedem | efried: left a comment also | |
| 21:30:40 | openstackgerrit | Dan Smith proposed openstack/nova master: Make instance able to lazy-load almost everything https://review.openstack.org/569267 | |
| 21:30:41 | openstackgerrit | Dan Smith proposed openstack/nova master: Expose instance_get_all_uuids_by_host() from DB API and use it https://review.openstack.org/569268 | |
| 21:31:56 | dansmith | mriedem: ^ | |
| 21:33:45 | dansmith | mriedem: we had pretty anal testing of the load behaviors for various special cases, | |
| 21:34:01 | dansmith | so keeping those tests unchanged and allowing the new case makes me fairly confident in that | |
| 21:35:18 | efried | dansmith: The "sentinel proj/user conf option" business was your idea, right? You wanna post a delta on the spec accordingly? | |
| 21:35:36 | efried | Maybe if we get enough hands on this spec, there'll be nobody left to approve it. | |
| 21:35:57 | dansmith | efried: (a) I don't like calling it a sentinel, but (b) I really wish we could just update the spec afterwards, because we've already wandered around a bunch of times | |
| 21:36:19 | efried | dansmith: But Matt will make fun of your haircut if we do that. | |
| 21:36:32 | dansmith | if people really need it then I will, but.. | |
| 21:36:37 | efried | mriedem: your call ^ | |
| 21:36:58 | dansmith | maybe I should so I can put "it's not an effing senintel people" in bold | |
| 21:37:05 | dansmith | that spec will be really yelly at that point | |
| 21:37:11 | dansmith | the angst is well codified | |
| 21:43:13 | mriedem | since i haven't gotten into the code yeah i'd like that | |
| 21:46:46 | efried | Either of y'all have any idea how allocation_ratio values are ending up 0.0 here? http://logs.openstack.org/32/569132/2/check/openstack-tox-py35/0ad1d4e/testr_results.html.gz | |
| 21:47:25 | efried | this test passes for me locally. | |
| 21:49:52 | efried | okay, I think I figured it out. There's another test *somewhere* stomping on those values. Just gotta track it down... | |
| 21:56:53 | mriedem | "gifts-for-mriedem" | |
| 21:56:55 | mriedem | that's nice | |
| 21:59:09 | dansmith | heh | |
| 22:02:37 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix interpretation of max_attempts for scheduling alternates https://review.openstack.org/569127 | |
| 22:05:37 | mriedem | dansmith: you missed a question i had in the earlier PS in ^ | |