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