14:00:10 <pdeore> #startmeeting glance 14:00:10 <opendevmeet> Meeting started Thu Jun 2 14:00:10 2022 UTC and is due to finish in 60 minutes. The chair is pdeore. Information about MeetBot at http://wiki.debian.org/MeetBot. 14:00:10 <opendevmeet> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 14:00:10 <opendevmeet> The meeting name has been set to 'glance' 14:00:10 <pdeore> #topic roll call 14:00:10 <pdeore> #link https://etherpad.openstack.org/p/glance-team-meeting-agenda 14:00:15 <pdeore> o/ 14:00:15 <abhishekk> o/ 14:00:19 <dansmith> o/ 14:00:25 <rosmaita> o/ 14:00:51 <mrjoshi> o/ 14:01:13 <pdeore> lets wait few minutes for everyone to join 14:01:34 * abhishekk There is intermittent electricity failure at my end since afternoon, I might get disconnected in between 14:02:10 <jokke_> o/ 14:02:26 <croelandt> o/ 14:02:28 <pdeore> let's start :) 14:02:32 <pdeore> #topic release/periodic jobs 14:02:48 <pdeore> We have skipped glance M1 tag due to gate issues 14:03:14 <pdeore> Periodic Jobs, all green except the 'oslo master' and 'translation update' jobs failure due to some python version dependency conflicts 14:03:31 <abhishekk> ack 14:04:05 <pdeore> moving ahead 14:04:11 <pdeore> #topic intermittent failure on glance-multistore-cinder-import job 14:04:16 <pdeore> rosmaita, ^ 14:04:30 <rosmaita> thanks 14:04:38 <rosmaita> abhishekk left a comment 14:05:02 <rosmaita> i'll see if we can figure out what the race condition could be 14:05:13 <rosmaita> it's intermittent, so not a big deal ATM 14:05:13 <abhishekk> Yes, I have tried it to reproduce locally but not able to do it 14:05:20 <abhishekk> https://github.com/openstack/glance/blob/master/glance/api/v2/image_data.py#L140 14:05:27 <abhishekk> this is where it is failing sometime 14:05:48 <rosmaita> ok, that is helpful 14:05:53 <rosmaita> that's all i have 14:06:07 <pdeore> ack, moving ahead 14:06:10 <pdeore> #topic Spec for review 14:06:16 <abhishekk> rosmaita, let me know if anything is needed 14:06:26 <rosmaita> ty 14:06:31 <pdeore> review request again for the location APIs spec (whoami-rajat) https://review.opendev.org/c/openstack/glance-specs/+/84088 14:06:45 <whoami-rajat> hey 14:06:50 <jokke_> yes, my bad I missed the updates on that earlier 14:07:00 <jokke_> on my todo list for today 14:07:06 <jokke_> been bit hectic 14:07:13 <whoami-rajat> jokke_, ack thanks! 14:07:15 <dansmith> whoami-rajat: is that marked as draft? 14:07:26 <dansmith> oh, no that url must be wrong 14:07:34 <whoami-rajat> dansmith, i think it should be active 14:07:38 <dansmith> pdeore: ^ 14:07:49 <rosmaita> missing a digit, i think 14:07:54 <dansmith> yeah 14:07:58 <whoami-rajat> https://review.opendev.org/c/openstack/glance-specs/+/840882 14:07:58 <dansmith> ends in -2 14:07:59 <pdeore> oops!! 14:07:59 <pdeore> https://review.opendev.org/c/openstack/glance-specs/+/840882 14:08:04 <whoami-rajat> #link https://review.opendev.org/c/openstack/glance-specs/+/840882 14:08:43 <whoami-rajat> wanted to request to at least remove the -2 so it doesn't discourage other reviewers from taking a look :) 14:08:46 <abhishekk> yep, I think we should follow a practice of avoiding putting -2 as per our core review guidelines 14:08:49 <whoami-rajat> i mean if the concerns are addressed ^^ 14:08:59 <dansmith> abhishekk: agree 14:09:11 <whoami-rajat> abhishekk, +1 14:09:13 <abhishekk> whoami-rajat, has pointed it out to me that we have these guidelines 14:09:13 <jokke_> whoami-rajat: yeah, will read it through and act accordingly 14:09:23 <jokke_> sorry for leaving it hanging for that long 14:09:41 <abhishekk> no worries 14:10:08 <pdeore> Spec for SRBAC system admin persona support in glance - https://review.opendev.org/c/openstack/glance-specs/+/844289 14:10:26 <jokke_> abhishekk: I say this again, feel free to remove me from core if you don't want me using my +-2 votes ;) 14:10:29 <abhishekk> I had a look at it and given some suggestions 14:10:45 <pdeore> I have submitted a spec for whatever we discussed so far on handling the owner issue for system scope 14:11:32 <jokke_> pdeore: saw that today. Will hopefully read through it too 14:11:58 <pdeore> abhishekk, I've addressed your comments, kindly have a look at new patch set 14:12:06 <abhishekk> jokke_, noted, I need to see everyone's interest is maintained 14:12:06 <pdeore> jokke_, thannks! 14:12:25 <abhishekk> pdeore, It needs some rewording, I will have a look at it once again 14:12:39 <pdeore> abhishekk, ack 14:13:07 <pdeore> Update proposal for duplicate image download - https://review.opendev.org/c/openstack/glance-specs/+/734683 14:13:21 <pdeore> this one also needs some reviews 14:13:50 <abhishekk> this is pending for long, I think rosmaita have some concerns/suggestions about it 14:15:07 <pdeore> ok, moving ahead 14:15:11 <rosmaita> i haven't looked at the latest version yet 14:15:11 <pdeore> #topic Are the version tests useless? 14:15:48 <pdeore> rosmaita, ^ 14:17:50 <rosmaita> sorry 14:18:34 <pdeore> np :) 14:18:35 <rosmaita> this is what i am talking about 14:18:40 <rosmaita> #link https://review.opendev.org/c/openstack/glance/+/843028 14:19:25 <dansmith> I would also add in that making the version we expose differ based on config is also not a great experience, which makes those tests far more complicated than need be 14:19:39 <abhishekk> rosmaita, afaik, this version increment is quiet messy at this moment and we need to sort it out 14:19:43 <rosmaita> yeah, and that's why i think they didn't break and detect the problem 14:20:20 <rosmaita> anyway, i guess my real point is that if you review anything that changes the version, make sure it is testing the right stuff 14:21:07 <rosmaita> ok, that's all from me, unless anyone has questions 14:21:08 <abhishekk> ++ 14:21:15 <pdeore> ++ 14:21:45 <dansmith> could we just discuss and move towards a single linear monotonically increasing version? 14:21:52 <jokke_> So say if it was just testing constants (not our first test doing so) I'd agree 100% now maybe only 90% :P 14:21:54 <dansmith> and stop the variation based on config? 14:23:05 <abhishekk> +1 for stopping increasing version based on config 14:23:28 <jokke_> dansmith: IIRC there is nothing that is based on config that is not already deprecated to be removed. All of the versions that are config dependant should be new features to be always on, but configurable at the beginning 14:24:03 <abhishekk> it is way more difficult to maintain these versions now as we have couple of config options in there 14:24:08 <dansmith> jokke_: okay, I think we had a couple things recently added that kept the same "if this is not enabled, then fall back in version" 14:24:28 <abhishekk> for multi store and image cache APIs 14:24:34 <dansmith> jokke_: but maybe we just need a comment in there that we should stop with the config-based versioning 14:24:44 <dansmith> and let the old stuff age out 14:24:45 <jokke_> well multi-store config is going away 14:25:07 <dansmith> https://review.opendev.org/c/openstack/glance/+/843028/2/glance/api/versions.py 14:25:20 <dansmith> the image_cache_dir controls which version we expose, for example 14:25:21 <abhishekk> not until we will be stopping single store support which will take couple of cycles atleast 14:25:46 <dansmith> and even still, you can't request the old version and get the old behavior (AFAIK) so I don't know why we wouldn't just collapse everything that is there right now 14:25:56 <rosmaita> the version negotiation will 404 if someone requests /v2.15/whatever and it's not enabled 14:25:56 <jokke_> abhishekk: my point, it's not planned to be there for ever, just ended up being there longer than we hoped for 14:25:56 <dansmith> and expose 2.16 as the current 14:26:37 <dansmith> rosmaita: but it doesn't mean anything right? it's silly to say 2.13 is enabled and 2.16 is enabled, but 2.15 is disabled 14:26:52 <rosmaita> well, it sort of gives you some info 14:26:52 <jokke_> so I've never understood the need for minor version API entrypoints in first place as we're not microversioning anything 14:27:01 <jokke_> it's just indicator 14:27:26 <jokke_> So it shouldn't be headace to anyone either just always call v2/foo/bar and you're grand 14:27:36 <abhishekk> jokke_, right, the point is it is difficult to maintain and to much confusing even for me to address new version bump if required 14:28:50 <dansmith> rosmaita: if you're saying they're supposed to be feature flags, I think it's super confusing for them to have linear numbers, instead of names or uuids :) 14:29:11 <jokke_> I guess it's good question to the next user survey if anyone is actually looking at the version info ... iirc I was this great idea of having autogenerating client based on schemas that drove us to the api versioning in the first place 14:29:16 <dansmith> and the current/supported classification would make even less sense 14:29:17 <rosmaita> i'm not sure what i'm saying 14:29:28 <dansmith> okay :) 14:29:31 <rosmaita> but it does help to know what version of the api you are contacting 14:29:44 <rosmaita> because if it's train, there's stuff you won't be able to do 14:29:55 <dansmith> rosmaita: right, but if the top version comes and goes based on config, 14:29:58 <dansmith> then you don't know 14:30:14 <rosmaita> well, you know for the site you are contacting, right? 14:30:20 <dansmith> so if you're trying to be bug compatible with train, but you can't tell if it's train or ussuri because config changes it, then you're hosed 14:30:34 <jokke_> The minor API version is there to indicate change in the API ... we have releases without API version bump and we have ones with. That's all it is, a flag "smething in the API has changed since the previous" 14:30:44 <jokke_> so it's not feature flag per se 14:30:45 <dansmith> right, 14:30:55 <dansmith> and if we have one version in a release that is dependent on config, 14:31:07 <dansmith> then you could look like release N or N-1 depending on your config 14:31:41 <jokke_> we've had quite a few of them, clearing out as the config options gets finally removed that allowed disabling the features causing the bump 14:32:43 <dansmith> well, perhaps a topic for the next ptg 14:32:50 <jokke_> I think the most confusing part is we have no documentation mapping what feature caused which version bump, so people really can't figure any of that out without looking git logs 14:33:07 <dansmith> jokke_: well, then let's torch it for sure :) 14:33:09 <abhishekk> I think to be short term solution we should carefully review a version bump patch, long term create a survey to check if version info is used or not and then follow some standard for version increase thereafter 14:33:30 <rosmaita> i left some comments on the patch that's up to fix the version history in the api-ref 14:34:03 <jokke_> I'd say if it's not useful lets get rid of the minor version in that api version all together and call it v2 like everyone using it :P 14:34:56 <jokke_> and we can just put a shim in place that routes /v2*/ -> /v2/ 14:36:01 <jokke_> So I'm not convinced the tests are useless, the minor version numbers might be :D 14:36:27 <rosmaita> well, it would be helpful to know what's new in the API 14:36:36 <rosmaita> https://review.opendev.org/c/openstack/glance/+/841053/2/api-ref/source/versions/index.rst#25 14:37:09 <jokke_> rosmaita: true, I wish we had something like concise notes for each release highlighting new features and changes 14:37:18 <jokke_> oh wait :P 14:39:57 <abhishekk> rosmaita, I think if owner is not around I will work on that patch 14:40:52 <rosmaita> that's ok with me 14:42:18 <abhishekk> I have noted this topic to be discussed in next PTG where probably we will meet in person 14:43:30 <pdeore> cool, shall we move to next topic ? 14:43:34 <abhishekk> lets move ahead for now 14:43:36 <rosmaita> in fabulous columbus, ohio!!! 14:43:42 <abhishekk> yep 14:44:04 <pdeore> moving to open discussion 14:44:22 <pdeore> #Open Discussion 14:44:22 <croelandt> Could Core take a few minutes to look at stable patches? :) 14:44:30 <croelandt> I think we can abandon the Victoria one 14:44:36 <croelandt> and there are a few that could use some +2s 14:44:48 <dansmith> croelandt: s/core/stable core/ :) 14:44:49 <abhishekk> dansmith, I have pushed new revision to immediate caching of images 14:45:05 <croelandt> dansmith: yes :) 14:45:07 <dansmith> abhishekk: just recently I assume? I looked yesterday and it was still failing 14:45:16 <abhishekk> its still WIP though 14:45:22 <abhishekk> not fixed tests atm 14:45:26 <dansmith> oh okay 14:45:53 <abhishekk> just wanted to make sure that approach is OK before moving to fix tests 14:46:02 <dansmith> okay I'll look 14:46:09 <abhishekk> croelandt, we can abandon victoria patches 14:46:27 <abhishekk> I will have a look at others today or tomorrow morning 14:46:31 <croelandt> ok ok 14:46:31 <abhishekk> dasm|off, thank you 14:47:01 <abhishekk> I am not able to found victor or the other member who were working on glance-download import method 14:47:08 <abhishekk> alistarle, around? 14:47:43 <abhishekk> dansmith, thank you 14:48:13 * abhishekk tagged different handle by mistake 14:49:42 <pdeore> anyone has anything else to discuss? or we are done for today ? :) 14:50:35 <jokke_> I have nothing, thnx 14:50:53 <abhishekk> nothing from me as well 14:51:13 <abhishekk> pdeore, just follow up for devstack multistore related patches with devstack team 14:51:31 <pdeore> abhishekk, sure 14:51:54 <abhishekk> yeah, we need to get it merged so that we can start work towards removing single store support 14:52:16 <alistarle> Hi, I see you comment on the glance patch abishek 14:52:32 <abhishekk> alistarle, yes 14:52:39 <abhishekk> just added it now 14:52:59 <alistarle> sorry for that, indeed Pierre Samuel is currently in hollyday, he will be back next week 14:53:17 <abhishekk> ack, no worries 14:53:29 <abhishekk> let us know if you guys need any help 14:53:34 <alistarle> we just need to write tests normally, this code is already in production on our side and seems to work well 14:54:09 <abhishekk> yeah, I have also given some reference to Pierre about coding standards which needs to be followed 14:54:13 <alistarle> yeah sure, you can say in the patch if there is anything you think is not right, we will fix it right away 14:54:31 <abhishekk> other than that I will have a look again at it 14:54:43 <alistarle> abhishekk have you seen something which are not following the guideline ? 14:55:22 <abhishekk> I don't remember it now, but will highlight it on the patch 14:55:37 <abhishekk> one thing is related to import 14:55:54 <abhishekk> where method name or exception class is imported which is not as per Openstack standard 14:56:23 <abhishekk> will highlight those on the patch 14:57:17 <abhishekk> last 3 minutes 14:57:25 <alistarle> yes please, we'll take care of that. And if you think the overall implementation is good we will fix and write the tests for the feature :) 14:57:52 <abhishekk> alistarle, ack, thank you 14:58:25 <pdeore> Thanks everyone for joining!! 14:59:32 <rosmaita> bye! 14:59:41 <jokke_> thanks all 15:00:01 <pdeore> #endmeeting