Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-12-15
16:49:15 gibi I'm meh on this. I think the new rules are meaningful and pretty cheap to introduce in placement so I'm happy to take them in
16:49:29 gibi introducing them to nova would make a lot of code change I guess
16:49:35 stephenfin I'd rather we kept the default list too
16:49:53 gibi stephenfin: default, meansh what is in nova's tox.ini?
16:49:55 stephenfin probably not worth the effort for nova, but the placement changes are done already
16:50:06 stephenfin no, what's in placement's tox.ini now
16:50:17 gibi yeah, I tend to agree with stephenfin
16:50:32 gmann yeah there are no other changes need in placement for those checks
16:50:35 bauzas my point is that we're changing placement code for the sake of a new hacking revision
16:50:52 bauzas I'd rather prefer to make nova and placement consistent in terms of style
16:51:16 bauzas besides the fact this would allow us to merge this change without needing to touch at the code
16:51:30 bauzas and later, we could just unignore rules one by one in placement if we really wish
16:51:47 bauzas baby steps, here, baby steps
16:52:00 bauzas 1/ upgrade hacking and ignore same list
16:52:16 bauzas 2/ unignore a specific rule and modify the code accordingly
16:52:27 bauzas 3/ iterate over every ignored rule
16:52:33 gibi bauzas: if you are after to split the hacking version change and the introdcution of new rules into separate patches then fine, but the current patch is not too big too
16:52:50 gmann bauzas: but this change with not-ignoring does not change much code
16:52:51 stephenfin if there were a significant amount of changes then I'd agree, but it doesn't seem worth the effort here
16:53:06 bauzas gibi: the problem is that we are silently changing placement without really explaining which hacking errors we wanted to fixc
16:53:29 gibi bauzas: sure, we can add a list of new rules applied to the commit message
16:53:40 bauzas and I don't think it's hard to just split this change in twice if you really want to unignore E128 and all the other rules
16:54:41 gibi OK, I'm fine in both ways: a) add a bit more text the commit message about the new rules that required the code change OR b) split the current patch into two
16:54:49 gmann giving it unblock the gate I preferred to move the code to latest hacking which is easy than doing one by one.
16:55:02 gibi gmann: agree, new hacknig is better
16:55:27 gmann I can add checks fix in commit msg.
16:55:28 stephenfin I don't think E128 is a big issue. W503/W504 seem to be the bulk of it. We could drop one of those, seeing as they conflict
16:55:49 gmann yeah W504 is ignored now
16:56:12 stephenfin sweet, then I don't care :)
16:56:39 gmann https://review.opendev.org/c/openstack/placement/+/766994/9/tox.ini
16:57:15 bauzas phew ok
16:57:25 bauzas I just feel we're overcomplicating
16:57:52 bauzas bumping hacking and providing the right ignore list will allow the gate to pass without needing to touch code
16:58:13 bauzas unless I'm missing something obvious
16:58:46 bauzas my take is, gmann had to touch code instead of amending the ignore list in order to fix the gate, I just feel it was unnecessary
16:58:51 stephenfin bauzas: not quite; a couple of the checks have been improved in newer versions of flake8 etc. To ignore those you'd have to disable the check and actually regress
16:59:13 bauzas stephenfin: why then nova isn't a problem ?
16:59:22 stephenfin because we fixed them there
16:59:27 stephenfin example: https://review.opendev.org/c/openstack/placement/+/766994/9/placement/conf/database.py
16:59:49 stephenfin that extra newline was (mistakenly) not required in an old version of flake8
17:00:27 bauzas for the extra newline, OK
17:00:39 bauzas for the continuous indentation, that's still there
17:00:55 bauzas so, ok, I mixed things
17:00:56 gmann nova is on 3.1.0. i could move placement too to that but 4.0.0 is already out to moving to it is easy.
17:01:26 bauzas gmann has to fix single lines within modules, but can just continue to leave uncontinuous indentation if he wishes
17:01:49 gmann anyways code is up and green so merging it is faster than splitting the patch now
17:02:53 bauzas well, I'm OK to remove my -1 for the sake of merging
17:03:01 bauzas but honestly, this is confisuing
17:04:12 gmann bauzas: how about if i explain those check fixes in cmt msg ?
17:04:43 bauzas I'm just saying we're again touching code for the beauty of our eyes and we're overcomplicating things, exactly like we did a couple of times in nova
17:04:50 bauzas but meh, I bail out
17:05:01 bauzas so, stephenfin can look at the change, I removed my -1
17:05:18 bauzas the +2 from gibi is there, so...
17:05:24 gibi yeah
17:05:34 gibi honestly I don't feel this as such a big issue
17:05:41 gibi but that jsutme
17:05:46 gibi just me
17:05:55 gmann bauzas: you seems do not like hacking checks much :)
17:06:19 bauzas it's more or less a matter of touching production code for the sake of ourselves being pedantic :)
17:06:28 bauzas but okay
17:07:15 bauzas gmann: well, I'm just pragmatic about things that work
17:07:26 bauzas but fair enough
17:08:18 gmann I see your point.
17:20:32 stephenfin ta, done
17:33:27 stephenfin gmann: Could you look at https://review.opendev.org/c/openstack/nova/+/765798/ at some point?
17:33:30 openstackgerrit Stephen Finucane proposed openstack/nova master: tests: Merge 'test_hypervisor_status' into 'test_hypervisors' https://review.opendev.org/c/openstack/nova/+/764039
17:33:31 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP: Change default policy for '/os-hypervisors' https://review.opendev.org/c/openstack/nova/+/765798
17:33:31 openstackgerrit Stephen Finucane proposed openstack/nova master: api: Drop statistics-style fields from os-hypervisors https://review.opendev.org/c/openstack/nova/+/764040
17:33:33 stephenfin I'm not sure if I'm tackling that correctly
17:33:43 stephenfin Also, resolved your comment on https://review.opendev.org/c/openstack/nova/+/764039
17:33:52 gmann stephenfin: sure, checking
17:36:29 openstack Launchpad bug 1906266 in OpenStack Compute (nova) "After upgrade: "libvirt.libvirtError: Requested operation is not valid: format of backing image %s of image %s was not specified"" [Undecided,New]
17:36:29 stephenfin lyarwood: you'd probably be interested in this at some point https://bugs.launchpad.net/nova/+bug/1906266
17:37:04 lyarwood stephenfin: didn't we fix that a while ago?
17:37:31 stephenfin yup, they're saying the fix is incomplete
17:37:52 gmann stephenfin: yeah that is right way, as system scope is disable by default and we still have old legacy rule in deprecated rule so we are good, - https://review.opendev.org/c/openstack/nova/+/765798/2/nova/policies/hypervisors.py#b37
17:38:08 lyarwood oh for existing images you actually need to add the backing format in, there's nothing n-cpu can do iirc
17:38:33 gmann stephenfin: no microversion needed for policy default, but please add releasenotes for that. I am checking other patches in that series
17:38:45 stephenfin gmann++ Great, thanks :)
17:38:54 lyarwood tbh I thought this was relaxed
17:39:21 openstackgerrit Sylvain Bauza proposed openstack/nova master: WIP: Add a routed networks scheduler pre-filter https://review.opendev.org/c/openstack/nova/+/749068
17:39:35 stephenfin maybe it was; they don't give any details about the versions they're using
17:43:46 stephenfin lyarwood: https://github.com/libvirt/libvirt/commit/ae9e6c2a2b75d958995c661f7bb64ed4353a6404
17:44:23 stephenfin the few patches I grepped from https://www.redhat.com/archives/libvir-list/2020-February/msg00616.html appear to be in v6.1.0
17:46:36 lyarwood ah!
17:46:46 lyarwood the error is still there but we don't get there now
17:47:02 stephenfin that's my reading of it too, yeah
17:50:25 ganso instructions in the placement docs are not very detailed to achieve what I need. Is anyone familiar with this use case or can think of any possible workarounds?
17:50:25 openstack Launchpad bug 1522307 in OpenStack Compute (nova) "duplicate for #1908133 Disk usage not work for shared storage" [Medium,Confirmed]
17:50:25 ganso Hi nova folks! Yesterday I logged bug https://bugs.launchpad.net/nova/+bug/1908133 and it has been closed as a duplicated. I'd like to discuss what possible workarounds there are (there are no mentions of workarounds in the duplicate entry). In my "steps to reproduce" I took 2 different approaches, the second one being relying on placement to control the shared allocations, but it is quite 100%, as I am not sure I am configuring it correctly. The
18:06:46 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP: Change default policy for '/os-hypervisors' https://review.opendev.org/c/openstack/nova/+/765798
19:13:42 jawad_axd Hi folks! Error 43, windows 10 with gpu passthrough. Added img_hide_hypervisor_id=True to image property. Is there anything I need to do to load nvidia drivers in windows instance?
19:14:10 jawad_axd *anything else
19:48:50 openstackgerrit Ghanshyam proposed openstack/nova master: Reuse code from oslo lib for JSON policy migration https://review.opendev.org/c/openstack/nova/+/763487
19:49:44 openstackgerrit Ghanshyam proposed openstack/nova master: Reuse code from oslo lib for JSON policy migration https://review.opendev.org/c/openstack/nova/+/763487
20:02:16 openstackgerrit Elod Illes proposed openstack/nova stable/ussuri: [stable-only] fix lower-constraints and disable qos resize https://review.opendev.org/c/openstack/nova/+/766738
20:44:11 openstackgerrit Merged openstack/nova master: libvirt: make cross cell resize spawn from snapshot image https://review.opendev.org/c/openstack/nova/+/765561
#openstack-nova - 2020-12-16
00:13:33 openstackgerrit Ghanshyam proposed openstack/placement master: Remove deprecated [placement]/policy_file config option https://review.opendev.org/c/openstack/placement/+/764409
00:13:57 openstackgerrit Ghanshyam proposed openstack/placement master: [goal] Deprecate the JSON formatted policy file https://review.opendev.org/c/openstack/placement/+/764394
00:15:14 gmann gibi: bauzas stephenfin please check these two placement patches also. for json->yaml migration community goal. https://review.opendev.org/c/openstack/placement/+/764409 https://review.opendev.org/c/openstack/placement/+/764394

Earlier   Later