| pdeore | #startmeeting glance | 14:00 |
|---|---|---|
| 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 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 14:00 |
| opendevmeet | The meeting name has been set to 'glance' | 14:00 |
| pdeore | #topic roll call | 14:00 |
| pdeore | #link https://etherpad.openstack.org/p/glance-team-meeting-agenda | 14:00 |
| pdeore | o/ | 14:00 |
| abhishekk | o/ | 14:00 |
| dansmith | o/ | 14:00 |
| rosmaita | o/ | 14:00 |
| mrjoshi | o/ | 14:00 |
| pdeore | lets wait few minutes for everyone to join | 14:01 |
| * abhishekk There is intermittent electricity failure at my end since afternoon, I might get disconnected in between | 14:01 | |
| jokke_ | o/ | 14:02 |
| croelandt | o/ | 14:02 |
| pdeore | let's start :) | 14:02 |
| pdeore | #topic release/periodic jobs | 14:02 |
| pdeore | We have skipped glance M1 tag due to gate issues | 14:02 |
| pdeore | Periodic Jobs, all green except the 'oslo master' and 'translation update' jobs failure due to some python version dependency conflicts | 14:03 |
| abhishekk | ack | 14:03 |
| pdeore | moving ahead | 14:04 |
| pdeore | #topic intermittent failure on glance-multistore-cinder-import job | 14:04 |
| pdeore | rosmaita, ^ | 14:04 |
| rosmaita | thanks | 14:04 |
| rosmaita | abhishekk left a comment | 14:04 |
| rosmaita | i'll see if we can figure out what the race condition could be | 14:05 |
| rosmaita | it's intermittent, so not a big deal ATM | 14:05 |
| abhishekk | Yes, I have tried it to reproduce locally but not able to do it | 14:05 |
| abhishekk | https://github.com/openstack/glance/blob/master/glance/api/v2/image_data.py#L140 | 14:05 |
| abhishekk | this is where it is failing sometime | 14:05 |
| rosmaita | ok, that is helpful | 14:05 |
| rosmaita | that's all i have | 14:05 |
| pdeore | ack, moving ahead | 14:06 |
| pdeore | #topic Spec for review | 14:06 |
| abhishekk | rosmaita, let me know if anything is needed | 14:06 |
| rosmaita | ty | 14:06 |
| pdeore | review request again for the location APIs spec (whoami-rajat) https://review.opendev.org/c/openstack/glance-specs/+/84088 | 14:06 |
| whoami-rajat | hey | 14:06 |
| jokke_ | yes, my bad I missed the updates on that earlier | 14:06 |
| jokke_ | on my todo list for today | 14:07 |
| jokke_ | been bit hectic | 14:07 |
| whoami-rajat | jokke_, ack thanks! | 14:07 |
| dansmith | whoami-rajat: is that marked as draft? | 14:07 |
| dansmith | oh, no that url must be wrong | 14:07 |
| whoami-rajat | dansmith, i think it should be active | 14:07 |
| dansmith | pdeore: ^ | 14:07 |
| rosmaita | missing a digit, i think | 14:07 |
| dansmith | yeah | 14:07 |
| whoami-rajat | https://review.opendev.org/c/openstack/glance-specs/+/840882 | 14:07 |
| dansmith | ends in -2 | 14:07 |
| pdeore | oops!! | 14:07 |
| pdeore | https://review.opendev.org/c/openstack/glance-specs/+/840882 | 14:07 |
| whoami-rajat | #link https://review.opendev.org/c/openstack/glance-specs/+/840882 | 14:08 |
| whoami-rajat | wanted to request to at least remove the -2 so it doesn't discourage other reviewers from taking a look :) | 14:08 |
| abhishekk | yep, I think we should follow a practice of avoiding putting -2 as per our core review guidelines | 14:08 |
| whoami-rajat | i mean if the concerns are addressed ^^ | 14:08 |
| dansmith | abhishekk: agree | 14:08 |
| whoami-rajat | abhishekk, +1 | 14:09 |
| abhishekk | whoami-rajat, has pointed it out to me that we have these guidelines | 14:09 |
| jokke_ | whoami-rajat: yeah, will read it through and act accordingly | 14:09 |
| jokke_ | sorry for leaving it hanging for that long | 14:09 |
| abhishekk | no worries | 14:09 |
| pdeore | Spec for SRBAC system admin persona support in glance - https://review.opendev.org/c/openstack/glance-specs/+/844289 | 14:10 |
| 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 |
| abhishekk | I had a look at it and given some suggestions | 14:10 |
| pdeore | I have submitted a spec for whatever we discussed so far on handling the owner issue for system scope | 14:10 |
| jokke_ | pdeore: saw that today. Will hopefully read through it too | 14:11 |
| pdeore | abhishekk, I've addressed your comments, kindly have a look at new patch set | 14:11 |
| abhishekk | jokke_, noted, I need to see everyone's interest is maintained | 14:12 |
| pdeore | jokke_, thannks! | 14:12 |
| abhishekk | pdeore, It needs some rewording, I will have a look at it once again | 14:12 |
| pdeore | abhishekk, ack | 14:12 |
| pdeore | Update proposal for duplicate image download - https://review.opendev.org/c/openstack/glance-specs/+/734683 | 14:13 |
| pdeore | this one also needs some reviews | 14:13 |
| abhishekk | this is pending for long, I think rosmaita have some concerns/suggestions about it | 14:13 |
| pdeore | ok, moving ahead | 14:15 |
| rosmaita | i haven't looked at the latest version yet | 14:15 |
| pdeore | #topic Are the version tests useless? | 14:15 |
| pdeore | rosmaita, ^ | 14:15 |
| rosmaita | sorry | 14:17 |
| pdeore | np :) | 14:18 |
| rosmaita | this is what i am talking about | 14:18 |
| rosmaita | #link https://review.opendev.org/c/openstack/glance/+/843028 | 14:18 |
| 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 |
| abhishekk | rosmaita, afaik, this version increment is quiet messy at this moment and we need to sort it out | 14:19 |
| rosmaita | yeah, and that's why i think they didn't break and detect the problem | 14:19 |
| 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:20 |
| rosmaita | ok, that's all from me, unless anyone has questions | 14:21 |
| abhishekk | ++ | 14:21 |
| pdeore | ++ | 14:21 |
| dansmith | could we just discuss and move towards a single linear monotonically increasing version? | 14:21 |
| 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 |
| dansmith | and stop the variation based on config? | 14:21 |
| abhishekk | +1 for stopping increasing version based on config | 14:23 |
| 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:23 |
| abhishekk | it is way more difficult to maintain these versions now as we have couple of config options in there | 14:24 |
| 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 |
| abhishekk | for multi store and image cache APIs | 14:24 |
| dansmith | jokke_: but maybe we just need a comment in there that we should stop with the config-based versioning | 14:24 |
| dansmith | and let the old stuff age out | 14:24 |
| jokke_ | well multi-store config is going away | 14:24 |
| dansmith | https://review.opendev.org/c/openstack/glance/+/843028/2/glance/api/versions.py | 14:25 |
| dansmith | the image_cache_dir controls which version we expose, for example | 14:25 |
| abhishekk | not until we will be stopping single store support which will take couple of cycles atleast | 14:25 |
| 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 |
| rosmaita | the version negotiation will 404 if someone requests /v2.15/whatever and it's not enabled | 14:25 |
| 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 |
| dansmith | and expose 2.16 as the current | 14:25 |
| 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 |
| rosmaita | well, it sort of gives you some info | 14:26 |
| jokke_ | so I've never understood the need for minor version API entrypoints in first place as we're not microversioning anything | 14:26 |
| jokke_ | it's just indicator | 14:27 |
| jokke_ | So it shouldn't be headace to anyone either just always call v2/foo/bar and you're grand | 14:27 |
| 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:27 |
| 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:28 |
| 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 |
| dansmith | and the current/supported classification would make even less sense | 14:29 |
| rosmaita | i'm not sure what i'm saying | 14:29 |
| dansmith | okay :) | 14:29 |
| rosmaita | but it does help to know what version of the api you are contacting | 14:29 |
| rosmaita | because if it's train, there's stuff you won't be able to do | 14:29 |
| dansmith | rosmaita: right, but if the top version comes and goes based on config, | 14:29 |
| dansmith | then you don't know | 14:29 |
| rosmaita | well, you know for the site you are contacting, right? | 14:30 |
| 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 |
| 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 |
| jokke_ | so it's not feature flag per se | 14:30 |
| dansmith | right, | 14:30 |
| dansmith | and if we have one version in a release that is dependent on config, | 14:30 |
| dansmith | then you could look like release N or N-1 depending on your config | 14:31 |
| 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:31 |
| dansmith | well, perhaps a topic for the next ptg | 14:32 |
| 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:32 |
| dansmith | jokke_: well, then let's torch it for sure :) | 14:33 |
| 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 |
| rosmaita | i left some comments on the patch that's up to fix the version history in the api-ref | 14:33 |
| 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 |
| jokke_ | and we can just put a shim in place that routes /v2*/ -> /v2/ | 14:34 |
| jokke_ | So I'm not convinced the tests are useless, the minor version numbers might be :D | 14:36 |
| rosmaita | well, it would be helpful to know what's new in the API | 14:36 |
| rosmaita | https://review.opendev.org/c/openstack/glance/+/841053/2/api-ref/source/versions/index.rst#25 | 14:36 |
| jokke_ | rosmaita: true, I wish we had something like concise notes for each release highlighting new features and changes | 14:37 |
| jokke_ | oh wait :P | 14:37 |
| abhishekk | rosmaita, I think if owner is not around I will work on that patch | 14:39 |
| rosmaita | that's ok with me | 14:40 |
| abhishekk | I have noted this topic to be discussed in next PTG where probably we will meet in person | 14:42 |
| pdeore | cool, shall we move to next topic ? | 14:43 |
| abhishekk | lets move ahead for now | 14:43 |
| rosmaita | in fabulous columbus, ohio!!! | 14:43 |
| abhishekk | yep | 14:43 |
| pdeore | moving to open discussion | 14:44 |
| pdeore | #Open Discussion | 14:44 |
| croelandt | Could Core take a few minutes to look at stable patches? :) | 14:44 |
| croelandt | I think we can abandon the Victoria one | 14:44 |
| croelandt | and there are a few that could use some +2s | 14:44 |
| dansmith | croelandt: s/core/stable core/ :) | 14:44 |
| abhishekk | dansmith, I have pushed new revision to immediate caching of images | 14:44 |
| croelandt | dansmith: yes :) | 14:45 |
| dansmith | abhishekk: just recently I assume? I looked yesterday and it was still failing | 14:45 |
| abhishekk | its still WIP though | 14:45 |
| abhishekk | not fixed tests atm | 14:45 |
| dansmith | oh okay | 14:45 |
| abhishekk | just wanted to make sure that approach is OK before moving to fix tests | 14:45 |
| dansmith | okay I'll look | 14:46 |
| abhishekk | croelandt, we can abandon victoria patches | 14:46 |
| abhishekk | I will have a look at others today or tomorrow morning | 14:46 |
| croelandt | ok ok | 14:46 |
| abhishekk | dasm|off, thank you | 14:46 |
| abhishekk | I am not able to found victor or the other member who were working on glance-download import method | 14:47 |
| abhishekk | alistarle, around? | 14:47 |
| abhishekk | dansmith, thank you | 14:47 |
| * abhishekk tagged different handle by mistake | 14:48 | |
| pdeore | anyone has anything else to discuss? or we are done for today ? :) | 14:49 |
| jokke_ | I have nothing, thnx | 14:50 |
| abhishekk | nothing from me as well | 14:50 |
| abhishekk | pdeore, just follow up for devstack multistore related patches with devstack team | 14:51 |
| pdeore | abhishekk, sure | 14:51 |
| abhishekk | yeah, we need to get it merged so that we can start work towards removing single store support | 14:51 |
| alistarle | Hi, I see you comment on the glance patch abishek | 14:52 |
| abhishekk | alistarle, yes | 14:52 |
| abhishekk | just added it now | 14:52 |
| alistarle | sorry for that, indeed Pierre Samuel is currently in hollyday, he will be back next week | 14:52 |
| abhishekk | ack, no worries | 14:53 |
| abhishekk | let us know if you guys need any help | 14:53 |
| alistarle | we just need to write tests normally, this code is already in production on our side and seems to work well | 14:53 |
| abhishekk | yeah, I have also given some reference to Pierre about coding standards which needs to be followed | 14:54 |
| 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 |
| abhishekk | other than that I will have a look again at it | 14:54 |
| alistarle | abhishekk have you seen something which are not following the guideline ? | 14:54 |
| abhishekk | I don't remember it now, but will highlight it on the patch | 14:55 |
| abhishekk | one thing is related to import | 14:55 |
| abhishekk | where method name or exception class is imported which is not as per Openstack standard | 14:55 |
| abhishekk | will highlight those on the patch | 14:56 |
| abhishekk | last 3 minutes | 14:57 |
| 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 |
| abhishekk | alistarle, ack, thank you | 14:57 |
| pdeore | Thanks everyone for joining!! | 14:58 |
| rosmaita | bye! | 14:59 |
| jokke_ | thanks all | 14:59 |
| pdeore | #endmeeting | 15:00 |
| opendevmeet | Meeting ended Thu Jun 2 15:00:01 2022 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 15:00 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/glance/2022/glance.2022-06-02-14.00.html | 15:00 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/glance/2022/glance.2022-06-02-14.00.txt | 15:00 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/glance/2022/glance.2022-06-02-14.00.log.html | 15:00 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!