Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-05-12
10:07:23 bauzas stephenfin: do you have more context ?
10:07:26 stephenfin yes, E741 and F522 seem to be new
10:07:36 bauzas so we could ignore them first ?
10:07:46 stephenfin but...why?
10:07:50 ikla set in flavor?
10:08:03 ikla oh, in the alias?
10:08:17 ikla I get the same issue with one device
10:08:29 stephenfin the correct fix is for hacking to limit flake8 to a given minor version instead of the major version range it's using, but we need a new release of hacking for that
10:09:58 sean-k-mooney ikla: are you testing with a gpu
10:10:23 sean-k-mooney ikla: or do you have any specific errors int eh nova compute agent log
10:11:36 ikla no, these are network cards
10:12:37 ikla nothing in logs
10:12:39 sean-k-mooney ok then likely you if its not the numa issue the next thing to check woudl be the pci device type
10:12:52 sean-k-mooney do the nics support sriov?
10:13:13 ikla yes
10:13:30 sean-k-mooney if they dont have the capablity of SRIOV then the type will be type-PCI
10:13:30 ikla they are setup and I can see them in the pci list with lspci -nnn
10:13:35 ikla Virtual x 4
10:13:46 sean-k-mooney if they do then the PF will be type-PF and the VFs will be type-VF
10:13:58 sean-k-mooney you need to match the alis to the type
10:14:35 ikla thats what I did
10:14:45 sean-k-mooney ikla: by the way you only use that pci aliase for nics if you dont want them to be used with neutron
10:15:11 ikla yup
10:18:55 sean-k-mooney ok so your alias is something like this
10:19:03 sean-k-mooney | [pci]
10:19:05 sean-k-mooney | alias = '{
10:19:07 sean-k-mooney | "name": "QuickAssist",
10:19:09 sean-k-mooney | "product_id": "0443",
10:19:11 sean-k-mooney | "vendor_id": "8086",
10:19:13 sean-k-mooney | "device_type": "type-vf",
10:19:15 sean-k-mooney | "numa_policy": "preferred"
10:19:17 sean-k-mooney | }'
10:20:04 sean-k-mooney actully it shoudl be type-VF
10:20:11 gibi tony_su: could you make a small fix in https://review.opendev.org/#/c/725788 then I will +2 it
10:21:35 bauzas stephenfin: I mean, your change is a bit unclear
10:22:10 openstackgerrit Lee Yarwood proposed openstack/nova stable/stein: stable-only: skip volume backup tests in cellsv1 job https://review.opendev.org/727147
10:22:29 openstackgerrit Lee Yarwood proposed openstack/nova stable/rocky: stable-only: skip volume backup tests in cellsv1 job https://review.opendev.org/727148
10:23:17 openstackgerrit Lee Yarwood proposed openstack/nova stable/queens: stable-only: skip volume backup tests in cellsv1 job https://review.opendev.org/727150
10:24:42 bauzas stephenfin: so, IMO, we should add a new change ignoring the new errors, then having your own change modifyng what what needed and then also removing the error ignored line
10:25:23 bauzas stephenfin: so in case a new patch would be merged, we would still make sure we would ignore the new errors until you are sure that all of them are fixed
10:30:19 sean-k-mooney do we even need to do that
10:30:24 sean-k-mooney lets just cap it
10:30:49 sean-k-mooney then props a patch to uncap it and a patch that depends on that in nova
10:31:01 sean-k-mooney it should fail and we can iterate on that patch until it passes
10:31:04 stephenfin or, you know, fix the five things that have changed and move on with our lives
10:31:19 stephenfin these are the only changes necessary. I checked. I'll update the commit message shortly
10:31:21 sean-k-mooney stephenfin: you didnt explain why any of the five things needed to be fixed
10:31:29 stephenfin I'll update the commit message shortly
10:31:35 sean-k-mooney and several fo them had #noqa on them
10:31:40 sean-k-mooney so they should have been ignofred
10:31:41 stephenfin they already had noqa
10:31:47 sean-k-mooney yep
10:31:52 stephenfin check my replies
10:31:53 sean-k-mooney so it should not have been checking them
10:33:24 sean-k-mooney stephenfin: https://www.flake8rules.com/rules/E741.html does not feel like we should have it on by default
10:35:52 stephenfin sean-k-mooney: I'd really rather avoid that argument because those tend to be ratholes in nova. I'd much, much rather we just took what flake8 and hacking gave us and dealt with it.
10:36:33 sean-k-mooney arbitary rules like that one make code worse and less readable
10:36:55 stephenfin arbitrary rules like you're *never* allowed exceed 80 characters?
10:37:03 sean-k-mooney i agree it can be consuing a time but its not any worse then using any 1 lettter valiable
10:37:17 stephenfin one man's arbitrary rule is another's good idea
10:37:18 sean-k-mooney stephenfin: yes that has been demonstrated to make code less readable
10:37:25 sean-k-mooney and pep8 enforece 79
10:37:28 sean-k-mooney not 80
10:37:59 stephenfin there is evidence to suggest otherwise https://www.youtube.com/watch?v=wf-BqAjZb8M&t=260
10:38:23 stephenfin https://black.readthedocs.io/en/stable/the_black_code_style.html#line-length would be an informative read
10:38:38 stephenfin but this is exactly where I don't want to end up :D damn it
10:38:53 stephenfin gibi: ta
10:39:23 sean-k-mooney stephenfin: i have read the black style guide it argues against the 80 column limit
10:40:23 sean-k-mooney stephenfin its why it used a 80ish limit rahter then a fixed value.
10:40:37 stephenfin yup
10:41:00 stephenfin to be clear, my argument is against the strict limit
10:41:04 stephenfin 80ish is fine
10:41:13 stephenfin hence the emphasis on *never* above
10:45:48 openstackgerrit Brin Zhang proposed openstack/nova master: Optimize _create_and_bind_arqs logic in conducor https://review.opendev.org/726564
10:47:41 sean-k-mooney stephenfin: i left some more comments. some of the noqas canbe remvoed with light tweeks or we can need to document why.
11:19:07 openstackgerrit Qiu Fossen proposed openstack/nova-specs master: specify mac for creating instance https://review.opendev.org/700429
11:40:48 openstackgerrit Nalini Varshney proposed openstack/nova master: Add migration to make key field type VARBINARY in aggregate_metadata table, https://review.opendev.org/725522
11:45:27 openstackgerrit sean mooney proposed openstack/nova master: [DNM] testing with force_legacy_port_binding workaround https://review.opendev.org/724387
11:45:27 openstackgerrit sean mooney proposed openstack/nova master: add workaround to disable multiple port bindings https://review.opendev.org/724386
11:57:01 openstackgerrit Takashi Natsume proposed openstack/nova master: Remove six.reraise https://review.opendev.org/726898
12:06:07 huaqiang stephenfin: Maybe I should drop my thought of reordering your bp/use-pcpu-and-vpuc-in-one-instance patches. My original thought was the followings patches could be acceleated since my patches donot depend on all the features introduced in the proceeding patches.
12:06:58 huaqiang But the reorder will bring a lot of extra work
12:07:20 huaqiang I'll rebase my pathces by following your patches.
12:10:13 openstackgerrit Takashi Natsume proposed openstack/os-vif master: Remove egg_info in setup.cfg https://review.opendev.org/727173
13:05:13 openstackgerrit Takashi Natsume proposed openstack/nova master: Remove six.reraise https://review.opendev.org/726898
13:46:50 openstackgerrit Lee Yarwood proposed openstack/nova stable/stein: stable-only: skip volume backup tests in cellsv1 job https://review.opendev.org/727147
13:47:57 openstackgerrit Lee Yarwood proposed openstack/nova stable/rocky: stable-only: skip volume backup tests in cellsv1 job https://review.opendev.org/727148
13:48:02 dansmith jsuchome: hey, I +2d that spec and then realized you forgot to address one critical piece of feedback from the last version
13:48:16 dansmith jsuchome: *other* than that, I was going to be happy with it :)
13:48:48 openstackgerrit Lee Yarwood proposed openstack/nova stable/queens: stable-only: skip volume backup tests in cellsv1 job https://review.opendev.org/727150
13:59:06 jsuchome dansmith: I see, I messed up the test part. And the other part I did not address intentionally, I'm not sure it deserves detailed explanation in the spec
13:59:48 jsuchome dansmith: the 'significant changes' in glance.py are really only about executing extra download method under given conditions. And I think this is already covered
14:01:58 dansmith jsuchome: you're changing the base existing glance code more than just calling the rbd function and returning the way the plug point used to work, so I think it's pretty relevant
14:02:13 dansmith jsuchome: there's also no documentation about it in the code change, so I don't even know what the goal of the change is
14:03:05 dansmith jsuchome: it does't need to be detailed, but it should be something, IMHO.
14:04:10 jsuchome I tried to explain it already but now I see I forgot to post my changes ...
14:05:17 jsuchome dansmith: I actually added some comment to that part you are (probably?) mentioning, I assume you mean "Load chunks from the downloaded image file... " bit
14:05:26 jsuchome I've added some comments to the spec
14:05:57 dansmith yeah, so re-reading the code this morning, it looks like we used to not do the image signature verification for things downloaded with the per-scheme handler,
14:06:03 jsuchome (I mean: I've added some comments 1. to the code and 2. now I've also commented the spec)

Earlier   Later