| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-10 | |||
| 17:30:00 | dansmith | nova.tests.functional.notification_sample_tests.test_server_group.TestServerGroupNotificationSample.test_server_group_add_member | |
| 17:30:33 | dansmith | (as an example) | |
| 17:32:38 | mriedem | hmm | |
| 17:33:09 | mriedem | oh i see why | |
| 17:33:15 | mriedem | https://review.openstack.org/#/c/563375/35..36/nova/objects/instance_group.py@142 | |
| 17:33:27 | mriedem | that should be removed or changed to {} | |
| 17:33:40 | dansmith | oh | |
| 17:33:55 | dansmith | sho nuf | |
| 17:34:16 | mriedem | and then we can make the rules field on the payload object no longer nullable as well https://review.openstack.org/#/c/563401/26/nova/notifications/objects/server_group.py@68 | |
| 17:35:38 | dansmith | lemme write a test for that behavior | |
| 17:36:11 | mriedem | yeah that group = objects.InstanceGroup(rules={}) && self.assertEqual({}, group.rules) | |
| 17:42:42 | openstackgerrit | Dan Smith proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375 | |
| 17:42:43 | openstackgerrit | Dan Smith proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401 | |
| 17:42:44 | openstackgerrit | Dan Smith proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166 | |
| 17:42:45 | openstackgerrit | Dan Smith proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465 | |
| 17:42:46 | openstackgerrit | Dan Smith proposed openstack/nova master: Microversion 2.64 - Use new format policy in server group https://review.openstack.org/567534 | |
| 17:42:51 | dansmith | okay here we go | |
| 17:44:07 | mriedem | pep8 brotha | |
| 17:45:05 | dansmith | come on | |
| 17:45:33 | mriedem | want me to tag in? | |
| 17:45:56 | dansmith | oh that's not my fault.. no I can just fix and push easy | |
| 17:46:02 | mriedem | https://review.openstack.org/#/c/563401/27/nova/notifications/objects/server_group.py@68 | |
| 17:46:06 | mriedem | also ^ | |
| 17:48:18 | dansmith | do I need to re-run tests after snipping that out? | |
| 17:48:23 | dansmith | the nullable thing | |
| 17:48:51 | dansmith | ah maybe hash will change? | |
| 17:50:20 | melwitt | gibi, mriedem: my bug fix got stalled on the functional test. I wasn't able to recreate the bug in a functional test so far | |
| 17:53:01 | openstackgerrit | Dan Smith proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375 | |
| 17:53:02 | openstackgerrit | Dan Smith proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401 | |
| 17:53:03 | openstackgerrit | Dan Smith proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166 | |
| 17:53:04 | openstackgerrit | Dan Smith proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465 | |
| 17:53:05 | openstackgerrit | Dan Smith proposed openstack/nova master: Microversion 2.64 - Use new format policy in server group https://review.openstack.org/567534 | |
| 18:00:43 | mriedem1 | i assume bauzas is in his futbol chamber | |
| 18:10:11 | mriedem | dansmith: ok i'm +2 up that stack to the point of the rest api change, reviewing that during this game | |
| 18:11:16 | dansmith | mriedem: okay we might want to find someone else to do that policy one since it's like half mine now | |
| 18:11:21 | dansmith | but I'll hit the rest | |
| 18:13:58 | dansmith | mriedem: hmm, so I wonder if that notification one should have been squashed into the main servergrouppayload like the objects were? | |
| 18:14:23 | dansmith | I was just mechanically updating it but hadn't really thought about it | |
| 18:14:24 | mriedem | i thought about that yesterday | |
| 18:14:46 | mriedem | and thought the separate payload isn't bad since it's the singular policy and the rules, | |
| 18:14:56 | mriedem | which in the rest api you just get a singular policy and (eventually) rules | |
| 18:15:01 | mriedem | so if we squashed, | |
| 18:15:10 | mriedem | the notification payload would have policies (deprecated list), policy and rules | |
| 18:15:20 | mriedem | so the nested object for the payload seems ok to me | |
| 18:15:27 | dansmith | so this mirrors more of what the api does? | |
| 18:15:28 | mriedem | doesn't matter to me much either way | |
| 18:15:38 | mriedem | the api will be flat | |
| 18:15:52 | mriedem | well, sorry, it's not flat | |
| 18:15:53 | mriedem | https://review.openstack.org/#/c/567534/28/doc/api_samples/os-server-groups/v2.64/server-groups-get-resp.json | |
| 18:16:00 | mriedem | yes it mirrors the rest api | |
| 18:16:12 | dansmith | hmm | |
| 18:17:21 | mriedem | we don't have to model it that way in the notification or api of course | |
| 18:17:27 | mriedem | could all be flat | |
| 18:18:04 | mriedem | gibi wanted both the old policies field and new policy payload in the notification for compat since notification consumers can't request a specific versoin of the notification payload | |
| 18:18:08 | mriedem | but that's not the same in the rest api | |
| 18:18:19 | dansmith | yeah, I dunno | |
| 18:18:37 | dansmith | just not sure I see the point of the nesting in either the api or the notification object | |
| 18:18:43 | dansmith | it's not bad, it just seems unnecessary | |
| 18:18:50 | dansmith | just another object and hash to track | |
| 18:19:29 | dansmith | anyway, if you want to just steam on I'll play along | |
| 18:26:02 | mriedem | i'm ok with making them flat if we want | |
| 18:26:37 | mriedem | thinking about stuff we nest in the rest api for servers, those are things like security groups, volumes, ports, etc - things that have their own resources in the api | |
| 18:26:50 | mriedem | server group policies wouldn't count like that since they aren't separate resources | |
| 18:26:55 | dansmith | yeah | |
| 18:27:26 | mriedem | gibi is probably the only other person that would have an opinion and he's probably gone by now | |
| 18:28:14 | mriedem | should we just wait and ask yikun what he thinks? if he doesn't care, then we can flatify tomorrow | |
| 18:28:28 | dansmith | if you're cool with that | |
| 18:28:33 | mriedem | yeah i'm fine with it | |
| 18:28:49 | mriedem | need to review Kevin_Zheng's abort queued live migration stuff today anyway | |
| 18:29:06 | mriedem | i'll drop my +2 on yikun's notification patch | |
| 18:30:27 | dansmith | aight | |
| 18:30:32 | dansmith | I'll comment | |
| 18:31:54 | openstackgerrit | Eric Fried proposed openstack/nova master: Tighten up ReportClient use of generation https://review.openstack.org/556669 | |
| 18:33:35 | mriedem | me too | |
| 18:33:35 | mriedem | https://review.openstack.org/#/c/563401/28/nova/notifications/objects/server_group.py@41 | |
| 18:33:42 | mriedem | gibi: fyi ^ | |
| 18:46:05 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Use ironic-tempest-dsvm-ipa-wholedisk-bios-agent_ipmitool-tinyipa in tree https://review.openstack.org/581444 | |
| 18:48:37 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: Use ironic-tempest-dsvm-ipa-wholedisk-bios-agent_ipmitool-tinyipa in tree https://review.openstack.org/581445 | |
| 18:53:02 | melwitt | looking for a +W on this ironic driver change needed to solve a race during instance creates https://review.openstack.org/563722 | |
| 18:55:45 | efried | Looks like jaypipes, dansmith, and johnthetubaguy have reviewed ^ in the past, so I'll stay away for now. | |
| 18:56:11 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: unquiesce instance after quiesce failure https://review.openstack.org/581451 | |
| 19:04:14 | openstackgerrit | Eric Fried proposed openstack/nova master: Delete orphan nodes before updating resources https://review.openstack.org/579922 | |
| 19:04:20 | openstackgerrit | Ken'ichi Ohmichi proposed openstack/nova master: Avoid BadRequest error log on volume attachment https://review.openstack.org/581453 | |
| 19:09:34 | openstackgerrit | Eric Fried proposed openstack/nova master: Address nits from consumer generation https://review.openstack.org/577227 | |
| 19:09:38 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: unquiesce instance after quiesce failure https://review.openstack.org/581454 | |
| 19:11:41 | mriedem | 7 days left to submit talks for berlin.... | |
| 19:14:16 | melwitt | I just swapped the runways (a day late, apologies) https://etherpad.openstack.org/p/nova-runways-rocky if anyone would like to add log notes for the removed ones and if dansmith could please update the channel topic | |
| 19:18:06 | dansmith | the first one doesn't have the blueprint slug in it | |
| 19:18:18 | melwitt | argh, sorry | |
| 19:19:53 | melwitt | thanks | |
| 19:21:40 | melwitt | vdrok, jroll, TheJulia: is this patch needed for https://blueprints.launchpad.net/openstack/?searchtext=allow-reserved-equal-total-inventory ? it's in merge conflict https://review.openstack.org/565841 | |
| 19:22:28 | melwitt | link correction https://blueprints.launchpad.net/nova/+spec/allow-reserved-equal-total-inventory | |
| 19:26:57 | TheJulia | melwitt: I think jroll is the only person who can know for sure, it looks like it just ought to be abandoned based upon the discussion. I know jroll has been super busy as of recent. | |
| 19:27:24 | melwitt | TheJulia: ack, thanks | |
| 19:28:25 | jroll | melwitt: I'll look post-meeting, been meaning to get back to that | |
| 19:28:46 | melwitt | jroll: thx | |
| 19:28:54 | jroll | I'd say it's needed but not for that BP | |
| 19:31:19 | melwitt | jroll: thanks for confirming. I'll update https://etherpad.openstack.org/p/nova-rocky-blueprint-status to call out just the one patch as needing review | |
| 19:31:29 | jroll | ++ | |
| 19:31:34 | melwitt | (this one https://review.openstack.org/517921) | |