| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-17 | |||
| 15:01:34 | mriedem | i guess? | |
| 15:01:37 | mriedem | idk man, just pull it down | |
| 15:01:53 | mriedem | i'm enjoying my -1 stat padding though | |
| 15:01:59 | sdague | heh | |
| 15:02:15 | sdague | this is one of those areas where github is *so* much better than gerrit | |
| 15:03:32 | jgriffith | mriedem johnthetubaguy hey... just want to check with the two of you. For the client.get_highest_microversion call; would you prefer that we ONLY call that when a call for a method that requires/wants the version info is requested? | |
| 15:04:26 | jgriffith | johnthetubaguy I think that might be the point you were trying to make in the review comments, so rather than ALWAYS do it on init, provide an explicit method to query that info, and then also use that mechanism when calling things like the new attach/detach methods | |
| 15:04:30 | openstackgerrit | Sean Dague proposed openstack/nova master: Improve descriptions for hostId, host, and hypervisor_hostname https://review.openstack.org/446761 | |
| 15:05:35 | leakypipes | mhenkel: yup! | |
| 15:05:53 | johnthetubaguy | jgriffith: so if you get your detach patch, and try use the logic, I think that will clear things up. That needs 3.27 or it fails with "the world is upside down" error. Only attach needs the explcit call to check if 3.27 is supported and chose the old or new path. | |
| 15:06:00 | melwitt | that api-ref wording is a hot topic I see! | |
| 15:06:13 | mriedem | +2 | |
| 15:06:14 | mriedem | finally | |
| 15:06:19 | mriedem | welcome new contributor | |
| 15:06:29 | johnthetubaguy | melwitt: looking at what dansmith has done to your quota patches, I guess you are working through that? | |
| 15:06:31 | jgriffith | johnthetubaguy ok | |
| 15:06:35 | jgriffith | johnthetubaguy thanks | |
| 15:07:00 | dansmith | johnthetubaguy: yes, I made a mess, but I knew it was going to be | |
| 15:07:03 | melwitt | johnthetubaguy: yes, sorry about that. I'm going to finish fixing those up today. we split them into smaller chunks for easier review | |
| 15:07:25 | dansmith | melwitt: I can help if you want, but probably not very parallelizeable work | |
| 15:07:38 | melwitt | johnthetubaguy: on the one you reviewed today, that "destroy all by project" is just a plain destroy. that patch is just making Quotas object read/write to API DB | |
| 15:08:26 | melwitt | dansmith: nah, it's okay, I have got it | |
| 15:09:14 | melwitt | johnthetubaguy: I think what you were thinking of was an older version of the quota counting patch where I was using a destroy with limit/offset for addressing races | |
| 15:09:18 | melwitt | I took that out | |
| 15:12:43 | openstackgerrit | Silvan Kaiser proposed openstack/nova master: Removes getfattr from Quobyte Nova driver https://review.openstack.org/428646 | |
| 15:13:05 | melwitt | johnthetubaguy: nevermind my earlier statement. I think I don't understand what you mean by "Its the delete something and the sync logic brings it back to life problem." | |
| 15:14:48 | melwitt | everybody was +2 on this patch PS8. all I did was update a few code comments based on leakypipes feedback | |
| 15:15:48 | leakypipes | melwitt: the Quota object patch you mean? | |
| 15:16:14 | melwitt | leakypipes: yeah, https://review.openstack.org/#/c/410945/ | |
| 15:16:23 | edleafe | kashyap: mdbooth: Sorry, wasn't able to grab the logs from the client machine - it's behind several layers of firewalls, so I'll need someone from their location to send them to me | |
| 15:16:26 | edmondsw | mriedem fyi just marked https://bugs.launchpad.net/nova/+bug/1656056 as a dup of https://bugs.launchpad.net/nova/+bug/1657774 where the fix is already proposed | |
| 15:16:26 | openstack | Launchpad bug 1657774 in OpenStack Compute (nova) "duplicate for #1656056 Nova does not re-raise 401 Unauthorized received from Neutron for admin users" [Undecided,Confirmed] - Assigned to Roman Podoliaka (rpodolyaka) | |
| 15:16:27 | openstack | Launchpad bug 1657774 in OpenStack Compute (nova) "Nova does not re-raise 401 Unauthorized received from Neutron for admin users" [Undecided,Confirmed] - Assigned to Roman Podoliaka (rpodolyaka) | |
| 15:16:38 | edleafe | They're in Japan, so it won't be until Monday | |
| 15:16:51 | edmondsw | mriedem you had marked the original high, so you probably want to do the same on the other now that the original is a dup | |
| 15:17:01 | edmondsw | fix proposed lgtm | |
| 15:17:26 | melwitt | leakypipes: I think I addressed all your comments, the one you wanted most was the single db transaction thing and I posted a comment that when I tried that, I got a "transaction already started" because I think the decorator already starts and maintains a transaction? | |
| 15:17:30 | kashyap | edleafe: No worries. When you have it, can take a look. But please don't forget to enable the log filters I noted in the scrollback | |
| 15:18:06 | leakypipes | melwitt: right, I think I had re-commented "ok, yes, makes sense" on that (or similar) :) | |
| 15:18:21 | melwitt | leakypipes: oh, hm, lemme check again | |
| 15:18:32 | edmondsw | mriedem_afk ^ I was tagging mriedem... | |
| 15:19:29 | melwitt | leakypipes: you didn't reply on that, the last reply I see from you was about the lack of a parent-child relationship of the tables | |
| 15:19:51 | leakypipes | doh, melwitt it was a draft comment never submitted... fuug | |
| 15:20:55 | edleafe | kashyap: yes, I checked, and they were exactly as you said | |
| 15:21:02 | edleafe | oops, my manners | |
| 15:22:54 | melwitt | leakypipes: a-ha! | |
| 15:25:40 | johnthetubaguy | melwitt: yeah, I was trying to remember what I mean in there too | |
| 15:25:52 | melwitt | heh :) | |
| 15:26:35 | johnthetubaguy | melwitt: I think it was fine before, just trying to remember what I was worried before | |
| 15:42:17 | openstackgerrit | Thomas Stewart proposed openstack/nova master: Add lan9118 as valid nic for hw_vif_model property for qemu https://review.openstack.org/393489 | |
| 15:45:32 | jsheeren | hi all | |
| 15:46:25 | jsheeren | i'm seeing this in my nova-compute log since today: INFO oslo.messaging._drivers.impl_rabbit [-] A recoverable connection/channel error occurred, trying to reconnect: Socket close | |
| 15:46:48 | jsheeren | the nova-compute service get's marked down by the controllers for a minute or two, then it recovers | |
| 15:46:56 | jsheeren | has anyone seen this behaviour before, | |
| 15:46:57 | jsheeren | ? | |
| 15:48:20 | leakypipes | mhenkel: looks like you still need to address those unit test failures, though. let us know if you need assistance. | |
| 15:50:48 | sneti | johnthetubaguy, I've updated the base patch for policy. can you please check this: https://review.openstack.org/#/c/434842/ | |
| 15:52:54 | mriedem | superdan: some easy test fixups in https://review.openstack.org/#/c/447009/ | |
| 15:53:07 | efried | mriedem (aka Swamp Thing) your influence has spread: https://review.openstack.org/#/c/445667/2/nova_powervm/tests/virt/powervm/volume/test_iscsi.py@53 | |
| 15:53:13 | mriedem | superdan: our next release of novaclient is going to be a major version bump, so we'll want to backport that bug fix to stable/ocata | |
| 15:54:06 | mriedem | efried: cool | |
| 15:54:33 | mriedem | superdan: but then after the bug fix i think we should just remove those deprecated arguments | |
| 15:54:50 | efried | mriedem BTW, yesterday I looked through nova-powervm to see what it would take to retrofit the whole project accordingly. Yeeeaaaahhh, prolly not gonna do that wholesale any time soon. | |
| 15:55:12 | mriedem | retrofit for what? autospec? | |
| 15:55:15 | efried | yeah. | |
| 15:55:27 | efried | But at least we can implement it moving forward. | |
| 15:55:40 | efried | And the in-tree driver will be clean. | |
| 15:55:56 | mriedem | warning on autospec, claudiub has experience with that https://review.openstack.org/#/c/339232/ | |
| 15:56:01 | mriedem | it does take a performance hit on the test run | |
| 15:56:19 | superdan | mriedem: I guess I missed the point at which we started having stable branches for clients | |
| 15:56:25 | mriedem | superdan: years ago | |
| 15:56:48 | mriedem | so we could backport the fix to ocata and release 7.1.1 | |
| 15:56:52 | efried | That's interesting - the docs on autospec claim it's lazy out of the box. | |
| 15:56:59 | superdan | mriedem: regardless of when, I guess I missed it | |
| 15:57:02 | mriedem | so your customer can pick that up w/o the backward incompatible changes that will be in 8.0.0 in pike | |
| 15:57:45 | mriedem | we needed stable branches on all of the clients and libraries just to get fixes back, plus we needed it at times to unscrew the gate back before upper-constraints | |
| 15:59:16 | claudiub | ooh, autospec | |
| 15:59:20 | claudiub | i'm actually working on it | |
| 15:59:29 | claudiub | I want it to become a permanent thingy | |
| 16:00:05 | claudiub | mriedem: something like this should do the trick: https://github.com/testing-cabal/mock/pull/394 | |
| 16:00:33 | claudiub | there's still an issue with mock.patch autospecing, on which i'm working on. | |
| 16:00:53 | mriedem | cool | |
| 16:01:38 | efried | claudiub Second sentence here claims it's already lazy: https://docs.python.org/3/library/unittest.mock.html#autospeccing | |
| 16:02:02 | efried | claudiub So you've discovered that's a false claim under certain conditions? | |
| 16:02:21 | claudiub | it is a false claim. :) | |
| 16:02:42 | claudiub | efried: https://github.com/testing-cabal/mock/issues/393 | |
| 16:02:45 | efried | claudiub Down a page or so, more explicitly: "Because the speccing is done “lazily” (the spec is created as attributes on the mock are accessed) you can use it with very complex or deeply nested objects (like modules that import modules that import modules) without a big performance hit." | |
| 16:02:55 | efried | Imagine my disappointment. | |
| 16:03:12 | claudiub | i know, right? :) | |
| 16:04:04 | claudiub | anyways, the only thing that specing / autospecing does at the moment, is ensure that your mock only have the same attributes / members as the spec itself, raising an AttributeError if non-existent attributes are accesed. | |
| 16:04:11 | claudiub | so, there's at least that. | |
| 16:04:19 | openstackgerrit | Lee Yarwood proposed openstack/nova master: compute: Move detach logic from manager into driver BDM https://review.openstack.org/439520 | |
| 16:04:19 | openstackgerrit | Lee Yarwood proposed openstack/nova master: compute: Only destroy BDMs after successful detach call https://review.openstack.org/440693 | |
| 16:04:32 | efried | claudiub That issue doesn't look related to performance/laziness | |
| 16:04:33 | lyarwood | mdbooth: ^ remove_volume_connection behaviour should remain the same now | |
| 16:05:05 | melwitt | claudiub, efried: well, it's expensive when you go and autospec every possible attribute, which is what you're doing right? | |
| 16:05:07 | claudiub | efried: but, you can give my PR i've linked above a try. it fixes this issue. check the added unit tests. There are still some unit tests failing for pypy, because the unittests themselves are faulty. | |
| 16:05:13 | mdbooth | lyarwood: Thanks, will look. | |
| 16:05:50 | claudiub | melwitt: actually, the way mock works, is create a child mock when a mock's attrbute is accessed, and it wasn't accessed before, and then caches it. | |
| 16:06:25 | claudiub | so, what I've done, is ensure that that child mock is properly spec'ed when created. | |