14:00:28 <abhishekk> #startmeeting glance 14:00:29 <openstack> Meeting started Thu Apr 8 14:00:28 2021 UTC and is due to finish in 60 minutes. The chair is abhishekk. Information about MeetBot at http://wiki.debian.org/MeetBot. 14:00:30 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 14:00:33 <openstack> The meeting name has been set to 'glance' 14:00:35 <abhishekk> #topic roll call 14:00:44 <dansmith> o/ 14:00:47 <abhishekk> #link https://etherpad.openstack.org/p/glance-team-meeting-agenda 14:00:49 <abhishekk> o/ 14:01:01 <abhishekk> lets wait couple of minutes for others to join 14:01:25 <dansmith> hoping for at least rosmaita so I can bug him about reviews :) 14:01:45 <abhishekk> he will be around :D 14:01:45 <rosmaita> i should have some time today 14:01:49 <rosmaita> o/ 14:01:51 <jokke> o/ 14:02:00 <abhishekk> cool, lets start 14:02:09 <abhishekk> #topic Updates 14:02:21 <abhishekk> #link https://etherpad.opendev.org/p/xena-ptg-glance-planning 14:02:33 <abhishekk> So most of the topics are final 14:03:10 <abhishekk> RBAC related discussion will be on Wednesday, and cross-project discussion with cinder will be either on Thursday or Friday 14:03:51 <rosmaita> can we make it thursday? 14:04:04 <abhishekk> We could have couple of empty slots, so if you have anything in mind please add it to planning etherpad 14:04:21 <abhishekk> rosmaita, yep, Thursday then 14:04:57 <abhishekk> I will update the etherpad accordingly 14:05:28 <abhishekk> any questions? 14:05:47 <abhishekk> cool, moving ahead 14:05:50 <abhishekk> #topic release/periodic jobs update 14:06:06 <rosmaita> thanks! 14:06:22 <abhishekk> We are good with release, I will be tagging stable releases next week 14:06:50 <dansmith> will we do an rc2 for the bug fix that merged today? 14:06:57 <dansmith> or just backport after the release? 14:07:29 <abhishekk> I think backport should be better one 14:07:36 <dansmith> ack 14:08:28 <abhishekk> Periodic job, one failure due to mirroring 14:08:54 <abhishekk> else it is green 14:09:05 <abhishekk> moving ahead 14:09:15 <abhishekk> #topic glance/glance-store job 14:09:37 <abhishekk> So recently we added one job in glance-store to run glance functional tests 14:10:09 <abhishekk> as it is a voting job, it is failing on glance-store change 14:10:44 <abhishekk> and we can not add depends on as it requires depends on both the patches (glance-store as well as glance) which is not possible due to circular dependency issue 14:10:45 <jokke> that was the point of it, right? ;) 14:10:53 <dansmith> yeah, so what this shows, is that you're breaking the library 14:10:54 <abhishekk> glance_store: https://review.opendev.org/c/openstack/glance_store/+/782200 14:11:17 <dansmith> the voting of it is doing exactly what we want, IMHO, which is to make sure a glance_store change does not break glance 14:11:17 <abhishekk> yeah, so workaround is to skip those tests, right? 14:11:39 <dansmith> I'll have to look at the failure and cause, but I'd say no 14:11:54 <abhishekk> glance: https://review.opendev.org/c/openstack/glance/+/784748 14:12:06 <dansmith> yep, I've got them up and will look after my meetings this morning 14:12:36 <abhishekk> ack 14:12:52 <abhishekk> thank you 14:13:17 <abhishekk> moving ahead 14:13:32 <abhishekk> #topic Possible improvement for image upload speed 14:13:48 <abhishekk> #link https://bugs.launchpad.net/glance/+bug/1921953 14:13:50 <openstack> Launchpad bug 1921953 in Glance "The upload of image is slow" [Undecided,New] 14:14:16 <abhishekk> I think whoever added this topic didn't mentioned that this improvement is specific to swift store 14:14:53 <abhishekk> and neither he/she is around at the moment 14:14:54 <dansmith> and, 14:15:03 <dansmith> there's no link to the proposed patch that I can see, right? 14:15:17 <abhishekk> Nope; 14:15:19 <GlaciErrDev> Hi! Right 14:15:25 <GlaciErrDev> i'll prepare patch 14:15:49 <abhishekk> GlaciErrDev, I think this should go as improvement rather than bug 14:15:56 <dansmith> agree 14:16:00 <GlaciErrDev> Sure 14:16:04 <GlaciErrDev> Thanks 14:16:10 <abhishekk> so if possible please submit a design spec 14:16:20 <abhishekk> are you aware about glance-specs ? 14:16:26 <GlaciErrDev> no 14:16:41 <abhishekk> #link https://github.com/openstack/glance-specs 14:16:50 <GlaciErrDev> thank you 14:17:07 <abhishekk> ok, here you can find specific template and also refer other available specifications 14:17:07 <jokke> GlaciErrDev: so what changes are you actually proposing? Is it just swift store driver change to call some other function in swiftclient or something more complex? 14:17:28 <abhishekk> jokke, I guess former one 14:18:20 <abhishekk> GlaciErrDev, and if you still have some doubts we can sync after the meeting and I can explain you more about the specs 14:18:21 <GlaciErrDev> I've just used ThreadPoolExecutor to execute in parallel `put_object` from swift 14:18:57 <dansmith> GlaciErrDev: if you're talking about a new thread per block, that could cause glance memory usage to skyrocket during upload, which is why we should have a spec 14:19:12 <abhishekk> ++ 14:19:41 <GlaciErrDev> actually this call `manager.get_connection().put_object(...)` 14:19:52 <GlaciErrDev> I see 14:20:11 <jokke> Yeah, I'd be very cautious about that. While one might find good performance increase in single operation we need to make sure we stay stable and perform well when there is 10s of them inflight 14:21:01 <dansmith> yup 14:21:17 <GlaciErrDev> I've tested with one upload operation a time... 14:21:53 <abhishekk> GlaciErrDev, so first submit a spec, you can also add your results in the specs 14:22:20 <GlaciErrDev> abhishekk ok, will do it 14:22:31 <jokke> GlaciErrDev: like abhishekk said, put your findings and what you did in a spec (lite should be sufficient as there is no API change involved) and lets see if it makes sense 14:22:33 <abhishekk> If required I will walk you through it 14:23:07 <GlaciErrDev> abhishekk probably it will help me a lot 14:23:08 <abhishekk> cool, GlaciErrDev thank you, 14:23:14 <GlaciErrDev> Thanks! 14:23:26 <abhishekk> we will sync after the meeting 14:23:29 <abhishekk> moving ahead 14:23:49 <abhishekk> #topic XS reviews 14:24:04 <abhishekk> this must be Steap 14:24:32 <Steap> yeah 14:24:52 <abhishekk> stage is yours 14:25:03 <Steap> 1 got an XS review and I think abhishekk marked a bunch of bugs ready to close 14:25:10 <Steap> https://review.opendev.org/c/openstack/python-glanceclient/+/784465 is an old Py3 issue we missed 14:25:21 <Steap> it does not seem to be triggered, but we might want to be safe 14:26:30 <abhishekk> Steap, I have added comments on the bugs and haven't got any replies on them yet, so those one with no replies we can close them 14:26:43 <Steap> yeah there is a list at https://etherpad.opendev.org/p/glance-bug-tracker 14:26:49 <Steap> you left them 1 week, right? 14:27:25 <abhishekk> yeah 14:27:47 <abhishekk> I haven't said that on bug to revert within a week or else we will close it 14:27:55 <Steap> hehe I give them 1 month, you're not as nice as me 14:28:01 <abhishekk> :D 14:29:10 <Steap> so, -10 bugs \o/ 14:29:40 <abhishekk> yep, and there are some stable branch related bugs as well at the bottom 14:30:39 <abhishekk> So I guess that's it from this topic, please have a look at review pointed by Steap 14:30:46 <abhishekk> Moving to Open discussion 14:30:54 <abhishekk> #topic Open discussion 14:31:23 <abhishekk> Open question for rosmaita 14:31:34 <abhishekk> #link https://review.opendev.org/c/openstack/glance/+/783668 14:31:35 <rosmaita> ? 14:31:36 <jokke> Steap: abhishekk: about the socketIO ... where is that coming from? I can't seem to find it in the stdlib socket docs 14:31:48 <dansmith> just a poke for him to circle back to that so I can revise :) 14:32:17 <Steap> hm 14:32:19 <Steap> weird 14:32:23 <rosmaita> ok, consider me poked 14:32:50 <Steap> jokke: oh it's in the code for sure, but seems undocumented 14:33:42 <jokke> Steap: so it's not coming from the python-socketio package we do not depend on? 14:33:48 <abhishekk> jokke, I have just verified by dir(socket) 14:34:36 <Steap> jokke: no, it's socket.SocketIO 14:35:21 <abhishekk> #link https://review.opendev.org/c/openstack/glance/+/783893 14:35:37 <abhishekk> rosmaita, Steap I have fixed your comments/suggestions on above patch 14:36:00 <Steap> Thanks, I'll recheck 14:36:11 <abhishekk> cool 14:36:12 <jokke> abhishekk: 3.7.7 does not have it 14:36:21 <jokke> I just did dir(socket) there 14:36:43 <abhishekk> oh, I have 3.6.9 14:37:12 <Steap> really? 14:37:45 <jokke> I think e need test going with that patch which actually executes the codepath 14:38:13 <Steap> yeah I'm not sure when this code is triggered 14:38:21 <rosmaita> dansmith: commented on your patch 14:38:38 <jokke> sorry, my bad there is indeed SocketIO in 3.7.7 14:38:39 <Steap> python3 -c "import socket; print('SocketIO' in dir(socket))" 14:38:39 <Steap> True 14:38:46 <jokke> yup 14:38:51 <dansmith> rosmaita: thanks 14:41:09 <jokke> Steap: abhishekk: so that established, as there is no documentation of that do we know it actually works same way or at all? Back to the point, if we did not catch socket._fileobject missing, we likely should have a test touching thta codepath showing that the SocketIO actually works as intended 14:42:18 <abhishekk> Steap, could you please add the test ? 14:42:38 <Steap> Honestly, I'm not sure what this code does. My reasoning here is that _fileobject is 100% sure to crash on us, while SocketIO seems to be the usual fix in projects that went from py2 to py3 :) 14:43:19 <abhishekk> neither do I, I will try to work on the test then 14:43:45 <Steap> Maybe the code is never actually used and we can just remove it 14:43:48 * Steap likes to dream 14:43:55 <jokke> I'll put that into my list of things to look at too to figure out what we actually do there. Thanks! 14:44:06 <abhishekk> jokke, cool, thank you 14:44:16 <abhishekk> anything else? 14:44:31 <jokke> abhishekk: whoami-rajat: btw about that glance_store patch ... I missed my opportunity at the time 14:45:05 <abhishekk> anything to mention? 14:45:14 <dansmith> abhishekk: one more thing 14:45:18 <whoami-rajat> ah is it already discussed? sorry i missed it 14:45:41 <dansmith> I think rosmaita's type is fixed here: https://review.opendev.org/c/openstack/glance/+/783893 14:45:42 <dansmith> and hoping anyone else that had comments has done so, so can we merge? 14:45:45 <jokke> Is there no way to not break glance on that? (I'm not sure the test change apart from lots of extra mocking looked like there was logic change) if that breaks the store python API that basically would need major version bump on glance_store 14:46:09 <dansmith> jokke: it looks to me like it's just breaking the fact that glance is mocking the internals of glance_store 14:46:30 <dansmith> jokke: which isn't technically a violation, and more just a problem of glance_store not exporting a stable Fixture for tests I think 14:46:46 <dansmith> jokke: I think putting the glance test fix in front of the glance_store patch will probably make it work 14:47:08 <dansmith> but I need to test.. it's just that glance is providing the client mock that glance_store ends up using I think 14:47:16 <dansmith> but I just briefly looked over it here in the meetnig 14:47:19 <abhishekk> dansmith, regarding https://review.opendev.org/c/openstack/glance/+/783893 14:47:31 <whoami-rajat> just to give my thoughts, my patch changes mostly the whole functional flow of current glance cinder to a new one so all old methods used are invalid hence the test fails 14:47:39 <abhishekk> if there are no more objections by tomorrow then we can move ahead with this 14:47:46 <jokke> dansmith: yeah same, you might be right it was just los of changes in the tests but indeed looks like loads of mocking 14:48:00 <dansmith> jokke: yeah I think it's just the mocking, unfortunately 14:48:06 <dansmith> abhishekk: okay 14:48:48 <abhishekk> I think during X we should also try to put efforts in our tests to remove mocking as much as possible 14:49:30 <dansmith> abhishekk: you mean across the seam of glance/glance_store right? 14:49:38 <jokke> in that case I'd suggest to put a patch to skip that test in glance functional with heavy todo note, merge the _store path, release the store and drop the skip. Rather than drop the whole test job to get it merged 14:49:44 <whoami-rajat> I would like if we made that job non-voting and i can propose a WIP fix to glance if it breaks and we can merge that patch when glance store releases but that's just my thoughts to make my work easier 14:50:16 <dansmith> jokke: I really think we can make the glance tests account for both cases and then merge the glance_store patch after that 14:50:48 <dansmith> i.e without skips or disabling the test 14:50:56 <jokke> dansmith: that would be great if you want to put your testing/mocking wizardry into action. I wouldn't even dare to try ;P 14:51:18 <dansmith> I don't think it'll be that bad, but yes I'll be glad to try before we do something more drastic 14:51:30 <abhishekk> that will be great, 14:52:14 <jokke> Hihi ... what comes to testing and specially mocking axe and sledgehammer tends to be my finetuning tools :P 14:52:21 <abhishekk> Last option would be report a bug in LP, skip the tests, release glance-store and then fix the bug 14:54:04 <jokke> abhishekk: yeah, well anyways if Dan can figure out way not needing to do that, even better. But I'd just rather not disable the whole job cause it works as intended :P 14:54:27 <abhishekk> ack 14:54:52 <dansmith> I got it 14:54:54 <dansmith> will push up in a few 14:54:58 <jokke> <3 14:55:02 <abhishekk> cool 14:55:18 <abhishekk> anything else ? 14:55:31 * jokke needs to start designing "mockwizzard" pointy hat 14:55:46 <jokke> so we can send one to dansmith 14:55:59 <jokke> nothing else from me 14:56:12 <abhishekk> ok, time to wrap up 14:56:20 <abhishekk> thank you all 14:56:38 <abhishekk> #endmeeting