| *** dviroel|out is now known as dviroel | 11:37 | |
| rosmaita | #startmeeting cinder | 14:00 |
|---|---|---|
| opendevmeet | Meeting started Wed Jun 23 14:00:09 2021 UTC and is due to finish in 60 minutes. The chair is rosmaita. 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 'cinder' | 14:00 |
| rosmaita | #topic roll call | 14:00 |
| whoami-rajat | Hi | 14:00 |
| simondodsley | Hello | 14:00 |
| rosmaita | #action rosmaita - find out why rbd-iscsi-client is not mirrored on github | 14:00 |
| fabiooliveira | hi | 14:00 |
| sfernand | hi | 14:00 |
| tbarron | hi | 14:01 |
| eharney | hi | 14:01 |
| jungleboyj | Hello. | 14:01 |
| abishop | o/ | 14:01 |
| enriquetaso | hi | 14:01 |
| rosmaita | looks like a good turnout, let's get started | 14:02 |
| rosmaita | #link https://etherpad.openstack.org/p/cinder-xena-meetings | 14:02 |
| rosmaita | #topic announcements | 14:02 |
| rosmaita | since next week's meeting is the final meeting of June, it will be held in both video and irc | 14:03 |
| rosmaita | also, if you missed it on the mailing list, lseki has stepped down as a cinder core | 14:03 |
| rosmaita | #link http://lists.openstack.org/pipermail/openstack-discuss/2021-June/023206.html | 14:03 |
| rosmaita | and, spec freeze is on Friday | 14:04 |
| rosmaita | #link https://releases.openstack.org/xena/schedule.html#x-cinder-spec-freeze | 14:04 |
| sfernand | :( | 14:04 |
| e0ne | hi | 14:04 |
| rosmaita | hello ivan | 14:04 |
| hemna | hey | 14:05 |
| rosmaita | i'm going to change the order of stuff on the agenda | 14:05 |
| rosmaita | we will discuss | 14:05 |
| rosmaita | sqlite (should take 2 minutes) | 14:05 |
| rosmaita | 10 minutes on simon's black linter proposal | 14:06 |
| rosmaita | and then the rest on spec reviews | 14:06 |
| rosmaita | #topic Drop support for SQLite < 3.7 | 14:06 |
| rosmaita | #link https://review.opendev.org/c/openstack/cinder/+/788871 | 14:06 |
| rosmaita | basically, all the currently supported platforms for openstack (centos-8-stream and ubuntu-focal) support sqlite >= 3.7 | 14:07 |
| rosmaita | we have a check in the code for the case where someone is using an older version | 14:07 |
| rosmaita | proposal is to remove it | 14:08 |
| e0ne | sqlite is used only got tests, so it's ok to use any version which doesn't make our tests more complicated | 14:08 |
| eharney | this only affects unit tests, so, not much risk | 14:08 |
| e0ne | eharney: +1 | 14:08 |
| rosmaita | but i was thinking maybe someone working with like queens, say, might be using an older sqlite on their dev box | 14:08 |
| hemna | Train ? | 14:09 |
| rosmaita | i think queens is still in -em mode | 14:09 |
| rosmaita | i believe train's supported platforms have sqlite >= 3.7 | 14:09 |
| rosmaita | but i am not 100% sure | 14:09 |
| hemna | I think train was ubuntu 18.0x > ? | 14:10 |
| rosmaita | ok, let's do this ... if anyone can think of a reason *not* to do this, please leave a comment on the patch before Friday 1200 UTC | 14:11 |
| rosmaita | otherwise, i am inclined to go with e0ne and eharney on this | 14:11 |
| rosmaita | thanks! | 14:11 |
| hemna | +1 | 14:11 |
| rosmaita | ok, it is almost 14:12 now | 14:11 |
| e0ne | #link https://governance.openstack.org/tc/reference/runtimes/train.html | 14:11 |
| rosmaita | #topic discuss allowing `black` to be used as an auto-linter | 14:12 |
| rosmaita | simondodsley: that's you | 14:12 |
| e0ne | it's good to have it but we'll lost history | 14:12 |
| simondodsley | i like to use `black` as an auto-linter - it makes life super easy especially for seeing future changes | 14:12 |
| simondodsley | e0ne: I agree you loose history - how much of an issue is that? | 14:13 |
| e0ne | git blame is usesul, but I prefer to have black :) | 14:13 |
| simondodsley | It isn't 100% perfect, but it makes linting somuch quicker | 14:13 |
| rosmaita | i think for the main cinder code, losing history is definitely an issue | 14:13 |
| simondodsley | I have found a couple of weird things that `tox -e pep8` fails with when using `black` but they are very trivial | 14:14 |
| e0ne | if we decide to use it, let's adopt it's settings to keep current code style | 14:14 |
| eharney | i thought black was opinionated and didn't really have settings to adjust | 14:14 |
| hemna | I've used black in other projects, and it doesn't always produce the most readable code IMHO | 14:14 |
| simondodsley | I'm not saying it should be mandatory - yes it is opionated | 14:15 |
| hemna | eharney it is very opinionated | 14:15 |
| e0ne | eharney: it has some settings and/or ignore list | 14:15 |
| rosmaita | so we can see the differnces: | 14:15 |
| rosmaita | #link https://review.opendev.org/c/openstack/cinder/+/792462/ | 14:15 |
| hemna | It can produce stuff like this: https://github.com/craigerl/aprsd/blob/master/aprsd/messaging.py#L561-L573 | 14:15 |
| hemna | which I think sux0rs | 14:16 |
| eharney | see, about the first thing it changed on line 57 there is annoying | 14:16 |
| eharney | (breaking our standard formatting of help text on a new line after the option name) | 14:16 |
| hemna | I use black in my ham radio python project called aprsd ^^ link above, as a mechanism to ensure that non python people can lint format their code before commit. | 14:16 |
| eharney | also not sure why it wants to change a lot of single-quoted strings to double quotes, weird | 14:17 |
| simondodsley | For new code is it a godsend, but true it does break some of the current standards, but are they critical? | 14:17 |
| rosmaita | yes, it definitely has its quirks | 14:17 |
| hemna | I think the function definition 'formatting' really sucks | 14:18 |
| eharney | i don't really see a benefit when pep8+hacking gets us quite consistent on style already | 14:18 |
| rosmaita | i like some of the stuff it does, but hate other things | 14:18 |
| jungleboyj | hemna: ++ | 14:18 |
| rosmaita | simondodsley: we really want a consistent style to make it easy to read any of the code files | 14:18 |
| jungleboyj | eharney: ++ | 14:18 |
| simondodsley | but using `pep8` requires lots of manual intervention and that is a waste of productive time | 14:18 |
| eharney | i just ran "black" on cinder/volume/manager.py and it blew up and told me to report a bug | 14:19 |
| simondodsley | I can `blacken` code in 5 seconds whereas manally editing for `pep8` compliance takes ages. | 14:19 |
| eharney | (was hoping to see what it did with type annotations) | 14:19 |
| simondodsley | `black` is a superset of `pep8` | 14:19 |
| geguileo | I don't care as much about the changing of standards as of losing history | 14:20 |
| hemna | another example of sux0rs...same problem though: https://github.com/craigerl/aprsd/blob/master/aprsd/main.py#L286-L301 | 14:20 |
| simondodsley | eharney: then you should :) | 14:20 |
| rosmaita | geguileo: ++ | 14:20 |
| eharney | i'd rather just stick to pep8/hacking style-wise | 14:20 |
| geguileo | eharney: and now you've seen what it does };-) | 14:20 |
| eharney | geguileo: indeed | 14:21 |
| rosmaita | i don't see the advantage of using this on the main cinder code because of the history issue | 14:21 |
| rosmaita | maybe for drivers? how do people feel about that? | 14:21 |
| simondodsley | would it be acceptable for new code? | 14:21 |
| rosmaita | or do we need to be completely consistent? | 14:21 |
| geguileo | maybe also for tests? | 14:21 |
| whoami-rajat | L#289-299 doesn't look good... | 14:21 |
| sfernand | git blame is critical for new devs, specially when trying to understand critical changes between os releases | 14:21 |
| e0ne | it doesn't make sense to have different code styles in the same repo | 14:21 |
| eharney | does black formatted code pass pep8/hacking most of the time? | 14:21 |
| rosmaita | sfernand: i agree | 14:22 |
| hemna | I think if a driver dev wants to run it on their driver and is ok with losing history that's one thing, still sucks for other people that touch those drivers, but running it on cinder core I think would not be good. | 14:22 |
| simondodsley | the style it creates make seeing code changes much simpler and makes for easier coding of changes going forward | 14:22 |
| geguileo | sfernand: and for old devs (like me) as well ;-) | 14:22 |
| simondodsley | I'm OK with loosing history for my driver code | 14:22 |
| hemna | also, backporting fixes to previous releases where black wasn't run.....sux0rs | 14:23 |
| rosmaita | i have been seeing some black-esque stylings in some patches recently | 14:23 |
| rosmaita | hemna: that is a really good point | 14:23 |
| geguileo | hemna: right!!! backports!!! | 14:23 |
| simondodsley | True - backports will break a lot. OK - I'll drop my thoughts on `black` for cinder | 14:23 |
| geguileo | I think we agree not to do it for core code and let driver maintainers choose whether to do it or not | 14:24 |
| jungleboyj | hemna: There is a huge issue. Backports. | 14:24 |
| e0ne | we know how to deal with backports. we have such rules for python3 only code | 14:24 |
| whoami-rajat | geguileo: but only on patches that they don't want to backport? | 14:24 |
| jungleboyj | I would prefer people don't use it. Definitely do not want it on core code. | 14:24 |
| e0ne | it's time for out-of-tree drivers discussion | 14:24 |
| jungleboyj | When I did the review I thought it sucked but went down the route of, 'Well, it is their driver' | 14:25 |
| geguileo | whoami-rajat: well, if they want to backport them they'll have to fix the merge conflicts | 14:25 |
| * jungleboyj does a hard eye roll | 14:25 | |
| rosmaita | ok, so it's looking like the consensus is definitely no black reformatting in the main code | 14:25 |
| tosky | do I understand it correctly the whole issue is due to black not being configurable to be tuned to produce the same style we use today? | 14:25 |
| hemna | e0ne lolz | 14:25 |
| rosmaita | ok, we are out of time on this | 14:26 |
| simondodsley | ok - let's say No - I'll refactor my patch | 14:26 |
| rosmaita | does someone want to take an action item to investigate black configuration? | 14:26 |
| rosmaita | so, for now ... no black | 14:26 |
| rosmaita | we can revisit if someone comes up with a way to address the points made above | 14:26 |
| rosmaita | thanks, simon | 14:26 |
| rosmaita | #topic xena specs review | 14:27 |
| rosmaita | well, it's spec freeze time again | 14:27 |
| jungleboyj | rosmaita: ++ | 14:27 |
| rosmaita | #link https://review.opendev.org/q/project:openstack%252Fcinder-specs+status:open | 14:27 |
| rosmaita | let' see what we got | 14:27 |
| hemna | hrmm user visible extra specs | 14:28 |
| rosmaita | i think the user-visible extra specs is in good shape, a revision was just pushed | 14:28 |
| hemna | I'm still not terribly sold on that | 14:28 |
| rosmaita | hemna: you need to act fast and review | 14:28 |
| hemna | ok will do. I don't think I'd want my customers to see my volume type extra specs. | 14:29 |
| hemna | it's very terse and driver specific | 14:29 |
| hemna | details that end users shouldn't see IMHO | 14:29 |
| rosmaita | well, it's going to be limited to a specific subset of extra-specs keys | 14:29 |
| rosmaita | defined in code, not configurable | 14:29 |
| abishop | hemna: driver specific specs are absolutely excluded from the feature | 14:29 |
| tbarron | hemna: they wont see any backend specific extra specs | 14:29 |
| hemna | I'll read through it | 14:29 |
| whoami-rajat | hemna: driver specific extra specs won't be visible | 14:29 |
| tbarron | backend-revealing | 14:30 |
| hemna | I'm just saying it's problematic for my customers | 14:30 |
| rosmaita | well, it will be governed by policy, so you can always turn it off for end-users | 14:30 |
| tbarron | and your customers can set policy to not show any if they want | 14:30 |
| hemna | wait, customers can set policy ?! or admins? | 14:31 |
| tbarron | admins | 14:31 |
| hemna | nm, I'll read the spec. | 14:31 |
| tbarron | i thought your customers were admins | 14:31 |
| hemna | no | 14:31 |
| hemna | I'm the admin :) | 14:31 |
| tbarron | if you are the admin then you do it if you want | 14:31 |
| rosmaita | anyway, give the spec a read, i think most of the issues have been worked out | 14:32 |
| rosmaita | nmve-of connection agent | 14:32 |
| rosmaita | #link https://review.opendev.org/c/openstack/cinder-specs/+/796365 | 14:32 |
| rosmaita | i think the spec captures the discussion at the PTG | 14:32 |
| rosmaita | so i think we're in agreement with the general direction | 14:33 |
| rosmaita | it will be useful for people to look it over for anything that needs to be spelled out more clearly | 14:33 |
| rosmaita | Spec for "Consistent and Secure RBAC" in Cinder | 14:33 |
| rosmaita | #link https://review.opendev.org/c/openstack/cinder-specs/+/797655 | 14:34 |
| rosmaita | we already agreed to do this, but i realized that we didn't have a spec to track it | 14:34 |
| rosmaita | so there it is | 14:34 |
| hemna | nice | 14:34 |
| rosmaita | eric: did your remove force from snapshots spec merge already? | 14:35 |
| hemna | I +A'd it this morning | 14:35 |
| rosmaita | cool | 14:35 |
| eharney | yes | 14:35 |
| rosmaita | i was worried there was a problem with my gerrit query | 14:35 |
| rosmaita | ok, the rest have -1s | 14:36 |
| hemna | I +A'd the temorary resource tracking spec as well | 14:36 |
| rosmaita | good | 14:36 |
| rosmaita | you have been busy this morning | 14:36 |
| hemna | yup :) | 14:36 |
| rosmaita | Support revert any snapshot | 14:37 |
| rosmaita | #link https://review.opendev.org/c/openstack/cinder-specs/+/736111 | 14:37 |
| rosmaita | there seems to be a failure of communication on this one | 14:37 |
| rosmaita | i asked the author to read the midcycle discussion | 14:38 |
| rosmaita | which stressed the importance of adding test scenarios to explain how this will work | 14:38 |
| rosmaita | but that hasn't happened | 14:38 |
| eharney | i think the spec is still missing info about safely handling operations that fail half-way through | 14:39 |
| rosmaita | ok, i will add a comment explaining more explicitly what needs to be addressed | 14:40 |
| rosmaita | it would be helpful if the proposer attended the cinder meetings so we could discuss | 14:41 |
| jungleboyj | rosmaita: ++ | 14:41 |
| rosmaita | Migration support for a volume with replication status enabled | 14:41 |
| rosmaita | #link https://review.opendev.org/c/openstack/cinder-specs/+/766130 | 14:41 |
| rosmaita | looks like geguileo has some concerns on this one | 14:41 |
| rosmaita | and it looks like the author is not willing to provide the details requested | 14:42 |
| geguileo | rosmaita: I don't know if I'm failing to explain what's missing | 14:43 |
| rosmaita | and those details are not unreasonable, they are key to understanding how this whole thing should work | 14:43 |
| eharney | the main concern is about migration would interfere with the replication, right? | 14:43 |
| rosmaita | this is another one that would have benefitted from discussion during the weekly meetings | 14:44 |
| geguileo | eharney: that is the primary, yes | 14:44 |
| geguileo | eharney: and iirc you also mentioned it before | 14:45 |
| rosmaita | Update original volume az | 14:45 |
| rosmaita | #link https://review.opendev.org/c/openstack/cinder-specs/+/778437 | 14:45 |
| rosmaita | this hasn't been updated since my midcycle comment, so not looking good | 14:45 |
| rosmaita | Allow volumes to be part of multiple volume groups | 14:47 |
| rosmaita | #link https://review.opendev.org/c/openstack/cinder-specs/+/792722 | 14:47 |
| rosmaita | i would've sworn that we had discussed it, but maybe not | 14:48 |
| geguileo | rosmaita: we did discuss it | 14:49 |
| geguileo | at least twice... | 14:49 |
| rosmaita | ok, i thought maybe i was dreaming about cinder again | 14:49 |
| geguileo | lol | 14:50 |
| rosmaita | ok, it hasn't been updated since 24 may, so i guess the proposer has lost interest | 14:50 |
| eharney | i'm concerned that this one is lacking some detail | 14:50 |
| whoami-rajat | rosmaita: I've left some comments on your srbac patch | 14:50 |
| geguileo | iirc the first time someone explained the use cases where this was useful | 14:50 |
| rosmaita | whoami-rajat: ty | 14:50 |
| eharney | it's not obvious what all the various things are that can happen when moving volumes in and out of consistency/replication groups | 14:50 |
| eharney | but i suspect it's more than just updating markers to say it's in those groups... | 14:51 |
| eharney | i.e. what happens with a chain of cgsnapshots when each snapshot has different volumes in it | 14:51 |
| rosmaita | ok, i'll send emails out about 736111, 766130, 778437, 792722 | 14:54 |
| rosmaita | the aim of the email being that they need to work with the team to address the issues | 14:55 |
| rosmaita | and if it's a TZ problem for the weekly meeting, we'll have to work something out | 14:55 |
| rosmaita | but i don't realistically see any of those being ready for Xena | 14:55 |
| rosmaita | ok, i guess that's it for specs | 14:56 |
| rosmaita | please review the ones mentioned at the beginning of this part of the meeting | 14:56 |
| rosmaita | they need to merge by 23:59 UTC or thereabouts on friday | 14:56 |
| rosmaita | any questions, comments? | 14:56 |
| jungleboyj | Question on the NVMe-oF | 14:56 |
| jungleboyj | https://review.opendev.org/c/openstack/cinder-specs/+/796365 | 14:57 |
| jungleboyj | So, There are +1s on it. No +2s . | 14:57 |
| jungleboyj | What do we want to do about that? | 14:57 |
| whoami-rajat | I've left some queries and suggestions (personal) to make it more readable | 14:58 |
| hemna | there is some good feedback on that review that's not been updated | 14:58 |
| rosmaita | i agree with Jay's comment that a more thorough os-brick person than me needs to give it a +2 | 14:59 |
| jungleboyj | hemna: Ok. That was why I wasn't +2. Just trying to interpret the other +1s | 14:59 |
| hemna | we had a discussion about this agent ages ago no? | 14:59 |
| hemna | I don't remember all of it | 15:00 |
| rosmaita | yes, at the PTG | 15:00 |
| rosmaita | #link https://wiki.openstack.org/wiki/CinderXenaPTGSummary#NVMe-oF_and_MDRAID_replication_approach_-_next_steps_for_connector_and_agent | 15:00 |
| hemna | I'll read through it | 15:00 |
| rosmaita | ok, thanks | 15:00 |
| rosmaita | like i said, i think it captures what we talked about | 15:00 |
| rosmaita | also, i am worried that the spdk CI has disappeared | 15:01 |
| rosmaita | although i guess that doesn't matter for this spec | 15:01 |
| rosmaita | ok, we are out of time ... thanks everyone, and please complete your spec reviews as soon as you can | 15:01 |
| rosmaita | #endmeeting | 15:02 |
| opendevmeet | Meeting ended Wed Jun 23 15:02:10 2021 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 15:02 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/cinder/2021/cinder.2021-06-23-14.00.html | 15:02 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/cinder/2021/cinder.2021-06-23-14.00.txt | 15:02 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/cinder/2021/cinder.2021-06-23-14.00.log.html | 15:02 |
| jungleboyj | Thanks! | 15:02 |
| rosmaita | that bot can't handle topics, but it's pretty snappy for start/end meeting! | 15:02 |
| whoami-rajat | thanks | 15:02 |
| whoami-rajat | :D | 15:02 |
| *** dviroel is now known as dviroel|out | 21:13 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!