| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-12-12 | |||
| 19:46:18 | mriedem | alright then | |
| 19:46:35 | dansmith | as you should | |
| 19:47:03 | dansmith | let it be remembered for all time, the last thing mriedem tried to do was an api-level hack for vmware | |
| 19:47:14 | dansmith | can you self paddle? | |
| 19:47:21 | mriedem | can a bear shit in the woods? | |
| 19:47:26 | dansmith | nevermind, don't answer | |
| 19:47:26 | dansmith | heh | |
| 20:00:33 | sean-k-mooney | hehe if only this was archive publicly for al time | |
| 20:00:44 | sean-k-mooney | *all | |
| 20:11:17 | KeithMnemonic | mriedem do you have a few to help me with a different marker issue. this one is in master as well and easily reproducible | |
| 20:12:40 | KeithMnemonic | not sure if it is just how horizon handles it or is it something in nova. under instances, there is not a prev/next button that came from this patch https://review.opendev.org/#/c/498018/34 | |
| 20:12:50 | KeithMnemonic | s/not/now | |
| 20:13:21 | KeithMnemonic | so that patch added a prev button, prior there was only next. but if you jump forward and back a few times you see it does not work as expected | |
| 20:14:09 | KeithMnemonic | seems to be due to the original list is not sorted or the sort is not maintained after doing 'next' than 'prev' | |
| 20:15:46 | KeithMnemonic | for example i have 5 instances test-1 through test-5. i set the "items per page" under admin settings to 2 | |
| 20:17:25 | KeithMnemonic | if i jump fresh to instances i see test-5, test-4, if i click next i get test-2, test-1. if i click prev, i get test test-3 and test-1 | |
| 20:17:44 | KeithMnemonic | so horizon is using marker= and prev_marker= and that seems to not work well | |
| 20:18:01 | KeithMnemonic | has anyone else seen this? | |
| 20:18:09 | mriedem | prev_marker isn't a thing in nova | |
| 20:18:20 | KeithMnemonic | so something horizon bonked? | |
| 20:18:38 | mriedem | maybe they aren't providing the full query params to maintain a consistent sort order, i.e. not passing the sort_key/sort_dir params when paging? | |
| 20:23:29 | KeithMnemonic | could be maybe somewhere in here https://opendev.org/openstack/horizon/src/branch/master/horizon/tables/base.py#L699 | |
| 20:25:09 | mriedem | idk | |
| 20:25:14 | mriedem | don't really have time to dig into that | |
| 20:26:14 | KeithMnemonic | no problem let me go bother horizon, thanks for confirming | |
| 20:26:57 | mriedem | but it does look like they are losing the sort key/dir | |
| 20:29:05 | efried | sean-k-mooney: https://review.opendev.org/#/c/674072/ +A, nice work on those tests. (See inline though.) | |
| 20:31:41 | mriedem | a couple of low-hanging bugs if someone wants something to play with | |
| 20:31:43 | mriedem | https://bugs.launchpad.net/nova/+bug/1856241 | |
| 20:31:43 | openstack | Launchpad bug 1856241 in OpenStack Compute (nova) "Compute API in nova - servers_links should link to paging doc" [Undecided,New] | |
| 20:31:46 | mriedem | https://bugs.launchpad.net/nova/+bug/1856240 | |
| 20:31:47 | openstack | Launchpad bug 1856240 in OpenStack Compute (nova) "wait_for_versioned_notifications gives unhelpful error message "ValueError: Not a text type application/octet-stream" on timeout" [Undecided,Confirmed] | |
| 20:57:10 | jroll | melwitt: re https://bugzilla.redhat.com/show_bug.cgi?id=1688838 - I feel like cpu_arch used to be required, but not 100%. looks like we intentionally made it not required here, at least: https://review.opendev.org/#/c/620634/ | |
| 20:57:11 | openstack | bugzilla.redhat.com bug 1688838 in openstack-nova "Ironic should not treat cpu_arch as mandatory" [Medium,New] - Assigned to mwitt | |
| 20:59:38 | openstackgerrit | Eric Fried proposed openstack/nova master: DRY: Build ImageMetaPropsPayload from ImageMetaProps https://review.opendev.org/698803 | |
| 20:59:47 | efried | dansmith: question in here ^ for you | |
| 21:00:00 | efried | sean-k-mooney: that ^ passes tests :) | |
| 21:01:58 | dansmith | efried: if the question is whether they need to be instances or classes, they need to be the former | |
| 21:02:21 | dansmith | the instance is what contains things like nullable=, default=, etc | |
| 21:02:34 | dansmith | and just because those don't have any of those set (currently) doesn't affect what the rest of the code is expecting | |
| 21:02:51 | efried | The question is whether they need to be discrete instances for every field of every object, or whether I could just copy in the instances from the other class. | |
| 21:03:30 | efried | like, could I theoretically do this: | |
| 21:03:30 | efried | f = IntegerField() | |
| 21:03:30 | efried | class MyObj(...): | |
| 21:03:30 | efried | fields = { | |
| 21:03:30 | efried | int1: f, | |
| 21:03:31 | efried | int2: f, | |
| 21:03:31 | efried | } | |
| 21:03:52 | dansmith | oh, well, I'll have to go look, | |
| 21:03:59 | dansmith | but the convention is obviously for each to be new | |
| 21:04:11 | dansmith | I can't remember if we stash any object-specific state in the field | |
| 21:04:19 | efried | Okay. Then prolly leave the code as it is now | |
| 21:05:12 | efried | thanks | |
| 21:06:23 | openstackgerrit | Eric Fried proposed openstack/nova master: DRY: Build ImageMetaPropsPayload from ImageMetaProps https://review.opendev.org/698803 | |
| 21:06:31 | efried | done, and changed the TODO to a NOTE | |
| 21:27:54 | sean-k-mooney | efried: oh i forgot the has. i just ran the test i added | |
| 21:27:58 | sean-k-mooney | *hash | |
| 21:28:16 | efried | sean-k-mooney: yahbut, see my fup | |
| 21:28:41 | efried | was going to say maybe you want to squash; but really they are quite different things, so let's keep them separate. | |
| 21:28:57 | efried | plus I don't want to lose the ability to +2 yours :P | |
| 21:29:22 | sean-k-mooney | yep i did i was wondering about that | |
| 21:29:34 | sean-k-mooney | if we do the follow up then we might as well nuke the object | |
| 21:29:56 | sean-k-mooney | well | |
| 21:31:23 | sean-k-mooney | we could replace the ImageMetaPayload 'properties': fields.ObjectField('ImageMetaPropsPayload') definition | |
| 21:32:02 | sean-k-mooney | with another filed that just reference the other object | |
| 21:32:24 | melwitt | jroll: thanks. I'm trying to figure out what's the correct thing to do in the nova ironic driver if ironic is not filling in a cpu_arch in a node. should we choose a default arch to report for ironic scheduling? if so, how should we choose it? a new config option? those are the kind of questions I'm trying to find answers about | |
| 21:35:06 | sean-k-mooney | efried: ok just read your comments on the patch below. ill create a fup to add the last combination and move the comments to doc strings afat i fix the Sync patch | |
| 21:35:51 | sean-k-mooney | and ya the ddt module is really nice to use. ill try to use it more often when it makes sense | |
| 21:37:13 | efried | sean-k-mooney: ++ | |
| 21:38:18 | efried | sean-k-mooney: Reasons not to nuke the object: it would make version boundaries weird; and I'm not sure what happens with SCHEMA. | |
| 21:38:32 | efried | It may be possible. But I'd leave it up to gibi | |
| 21:39:34 | sean-k-mooney | well with your follow up patch every time we add a filed to the normal nova object the notificaiton object fileds and schema will auto update chaning the hash | |
| 21:40:14 | sean-k-mooney | so if we apply that patch we need to rememebr to bump the notificion object version and document the new filed even though we are not modifying the class | |
| 21:40:33 | efried | ohhh, good point, the version. That needs a -1. | |
| 21:40:57 | sean-k-mooney | that is still less work then today however. ya i can put that in a review comment | |
| 21:41:28 | efried | Yeah, we at least need to make a note in the class with that instruction. | |
| 21:41:41 | efried | the test will fail, but you could get around it by just updating the hash without bumping the version. | |
| 21:41:52 | sean-k-mooney | yep | |
| 21:41:54 | efried | an astute reviewer ought to notice, but... | |
| 21:42:11 | jroll | melwitt: it feels like it should be a required field for an ironic node | |
| 21:42:20 | jroll | there's no way to guess at it | |
| 21:56:24 | sean-k-mooney | jroll: the cpu architecture | |
| 21:57:07 | sean-k-mooney | ya i mean some hyperisors can emulate other archs but that is kind of true for all virt drivers | |
| 22:00:32 | mriedem | ironic != hypervisor | |
| 22:00:54 | sean-k-mooney | also true | |
| 22:01:55 | sean-k-mooney | i was makeing the point that most of the virt driver only support one archtecture at a time so you amost always want to know the cpu architecture if you care about check the image for compatablity | |
| 22:03:30 | sean-k-mooney | there was that project denvor chip that nviad wanted to make but then never closed the license agreement with intel for x86 | |
| 22:04:41 | sean-k-mooney | iwas going to be an arm architecture chip with native traslation of x86 to arm opcode in hardware allowing them to run code comiled for either achtecture | |
| 22:05:21 | sean-k-mooney | im sure that would have been a pain to debug but it would have been cool if it ever worked | |
| 22:11:28 | openstackgerrit | sean mooney proposed openstack/nova master: Sync ImageMetaPropsPayload fields https://review.opendev.org/698613 | |
| 22:15:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use COMPUTE_SAME_HOST_COLD_MIGRATE trait during migrate https://review.opendev.org/695220 | |
| 22:23:39 | sean-k-mooney | efried: actully my brain has already switched off today so ill do the FUP tomorrow the other FUP i promised is https://review.opendev.org/#/c/698260/ but there is no really rush with reviewing that | |
| 22:24:01 | efried | ack | |
| 22:24:42 | openstackgerrit | Merged openstack/nova master: trivial: Resolve (most) flake8 3.x issues https://review.opendev.org/695732 | |
| 22:41:53 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add NodeOwnerFilter https://review.opendev.org/697331 | |
| 22:41:53 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Allow overriding self.api in _build_minimal_create_server_request https://review.opendev.org/698813 | |
| 22:51:58 | openstackgerrit | Merged openstack/nova master: Don't hardcode Python versions in test https://review.opendev.org/698732 | |
| 23:39:21 | openstackgerrit | Merged openstack/nova master: Keep pre-commit inline with hacking and fix whitespace https://review.opendev.org/674057 | |
| 23:42:35 | openstackgerrit | Merged openstack/nova stable/queens: Add functional regression test for bug 1849409 https://review.opendev.org/690724 | |
| 23:42:35 | openstack | bug 1849409 in OpenStack Compute (nova) queens "openstack server list --deleted --limit -1 hangs" [High,In progress] https://launchpad.net/bugs/1849409 - Assigned to Matt Riedemann (mriedem) | |