| *** dviroel|rover|afk is now known as dviroel|rover | 01:17 | |
| opendevreview | Gorka Eguileor proposed openstack/os-brick master: DNM: NVMe-oF temporary change for CI's to use https://review.opendev.org/c/openstack/os-brick/+/849891 | 07:56 |
|---|---|---|
| *** akekane_ is now known as abhishekk | 08:04 | |
| whoami-rajat | geguileo, hey, the copyright header is missing from another file as well ... https://review.opendev.org/c/openstack/os-brick/+/849324, let me know if you update and i will +2 again | 08:54 |
| elodilles | whoami-rajat: quick double check with the client / library zed-2 releases: could you have a quick look at them maybe? https://review.opendev.org/c/openstack/releases/+/849363 | 09:09 |
| elodilles | whoami-rajat: and this one: https://review.opendev.org/c/openstack/releases/+/849384 | 09:09 |
| elodilles | (yesterday was the deadline, so we are merging the zed-2 patches without responses) | 09:10 |
| elodilles | oh, and this one: https://review.opendev.org/c/openstack/releases/+/849382 | 09:11 |
| whoami-rajat | elodilles, sorry i discussed this in the cinder meeting for forgot to leave a response on the patches | 09:12 |
| whoami-rajat | elodilles, the os-brick release requires a patch, i will get that merged in some time | 09:13 |
| whoami-rajat | elodilles, other two should be good to go but i will take a quick look | 09:13 |
| opendevreview | Rajat Dhasmana proposed openstack/os-brick master: Support independent file lock path https://review.opendev.org/c/openstack/os-brick/+/849324 | 09:21 |
| whoami-rajat | geguileo, since it was one line change I've updated the patch to save Zuul run time ^ | 09:21 |
| whoami-rajat | rosmaita, once you come online, can you re-vote on this? https://review.opendev.org/c/openstack/os-brick/+/849324 -- needed for the z-2 brick release | 09:24 |
| geguileo | whoami-rajat: https://m.media-amazon.com/images/I/41YuyC+JtKL._AC_.jpg | 09:27 |
| geguileo | ^ that's a joke, btw | 09:27 |
| whoami-rajat | lol | 09:29 |
| elodilles | whoami-rajat: ok, no problem, please leave a -1 on the patch that needs rework, so that we won't merge that 'accidentally' :] | 09:32 |
| elodilles | ignore me, did not see that you -1'd it already :X | 09:42 |
| whoami-rajat | elodilles, yep, already left comments on all 3 release patches, thanks for the reminder :) | 09:53 |
| whoami-rajat | great, thanks | 10:27 |
| rosmaita | whoami-rajat: about https://review.opendev.org/c/openstack/os-brick/+/849324/ ... I think eharney | 11:17 |
| rosmaita | 's point was that it doesn't make sense to have "All Rights Reserved" if there's not copyright statement | 11:17 |
| rosmaita | the copyright statement is not mandatory | 11:17 |
| rosmaita | and the "All Rights Reserved" is not necessary if there's a copyright statement anyway | 11:18 |
| rosmaita | but the problem i have here is that the author should decide whether to add the statement or not | 11:18 |
| rosmaita | so it's not the kind of thing for someone else to push a patch | 11:18 |
| rosmaita | so it would be good to have a comment from geguileo that he's ok with that change | 11:19 |
| rosmaita | before approving it | 11:19 |
| whoami-rajat | rosmaita, the opts file had "All Rights Reserved" and no copyright statement so we had 2 choices, to include the copyright statement or remove the "All Rights Reserved" statement. I updated the patch as per the last change done i.e. to include the copyright statement | 11:20 |
| whoami-rajat | though i understand i shouldn't have updated it and it sets a bad precedent | 11:21 |
| rosmaita | yes, but as you are not the author, you cannot assert copyright morally | 11:21 |
| whoami-rajat | yes, totally agree | 11:21 |
| rosmaita | so if you had just removed "All Rights Reserved" that woudl be ok because there's no copyright statement | 11:21 |
| rosmaita | but adding it seems weird | 11:22 |
| rosmaita | btw, i believe this is still accurate: "Copyright Headers" in https://wiki.openstack.org/wiki/LegalIssuesFAQ | 11:22 |
| whoami-rajat | so we need geguileo to approve my change and we're good to go? | 11:25 |
| whoami-rajat | (it's obvious but approve with a comment and not vote) | 11:26 |
| *** dviroel|rover is now known as dviroel | 11:28 | |
| whoami-rajat | wow, the All rights reserved is not a recommended practice "Therefore it is recommended that developers not include "All rights reserved" in copyright headers." | 11:29 |
| geguileo | whoami-rajat: rosmaita so what do we want to do? | 12:08 |
| geguileo | do we want to remove it? or do I just wrote a message on the review saying thank you? | 12:10 |
| whoami-rajat | geguileo, i think what rosmaita want is you acknowledge and approve my change in reply to this comment https://review.opendev.org/c/openstack/os-brick/+/849324/5..6/os_brick/opts.py#b1 | 12:10 |
| whoami-rajat | a thank you would be good :D | 12:10 |
| rosmaita | message on the review is fine | 12:11 |
| geguileo | done | 12:11 |
| rosmaita | ty, sorry to be a PITA | 12:11 |
| whoami-rajat | it's good we do things in the right way so thank you for pointing it out | 12:12 |
| geguileo | this patch just needs a +W https://review.opendev.org/c/openstack/os-brick/+/836055 | 12:12 |
| rosmaita | geguileo: about the service-side changes, was your plan to keep the lambda failsafe in there, or remove it and depend on the correct version of os_brick being in requirements? | 12:12 |
| geguileo | rosmaita: Since you and smooney wanted it without the lambda I'll update the patches once we have a release | 12:14 |
| rosmaita | ok, cool | 12:15 |
| geguileo | rosmaita: whoami-rajat eharney Why do we have ceph only as voting for gate jobs but not for checks? | 12:28 |
| geguileo | s/ceph/ceph job | 12:28 |
| rosmaita | inertia? | 12:29 |
| rosmaita | do you mean for cinder? | 12:29 |
| geguileo | yup | 12:29 |
| geguileo | This patch can't merge until we release a new os-brick version in Yoga: https://review.opendev.org/c/openstack/cinder/+/848821 | 12:29 |
| geguileo | because we run jobs with released os-brick and os-brick on ceph is broken in Yoga | 12:29 |
| geguileo | so basically we cannot merge Cinder patches in Yoga right now | 12:30 |
| geguileo | (because of me) | 12:30 |
| rosmaita | geguileo: i am probably misunderstanding what you're asking, cinder-plugin-ceph-tempest shows as voting in both check and gate in https://opendev.org/openstack/cinder/src/branch/stable/yoga/.zuul.yaml | 12:34 |
| geguileo | rosmaita: oh, I thought both where non-voting! but it's only the multinode a-a | 12:36 |
| rosmaita | although looking at that, i wonder if we are being too aggressive with the gate-irrelevant-files | 12:36 |
| rosmaita | it doesn't run on requirements changes | 12:36 |
| rosmaita | on the theory that problems with those would be caught by unit tests | 12:36 |
| rosmaita | and functional tests | 12:37 |
| rosmaita | which i guess is probably ok? | 12:37 |
| geguileo | mmmm, probably not great... | 12:38 |
| rosmaita | i think we didn't want to waste resources, but maybe that shouldn't be a worry | 12:38 |
| geguileo | do we run any tempest test when reqs change? | 12:39 |
| rosmaita | no, i think they all use that same set of gate-irrelevant-files | 12:40 |
| rosmaita | maybe we should pick one, maybe tempest-integrated-storage, and run it on everything except doc changes | 12:41 |
| tosky | (tempest-integrated-storage doesn't run c-bak tests) | 12:47 |
| rosmaita | tosky: good point ... what would you suggest as a canary tempest job to run on requirements changes? | 12:48 |
| rosmaita | i guess we could always add a special job for that that runs everything! | 12:49 |
| tosky | we have our ows, the lio-lvm-barbican and the ceph-cbak | 12:52 |
| tosky | the former looks more stable | 12:52 |
| rosmaita | what do you think of changing the irrelevant-files on cinder-tempest-plugin-lvm-lio-barbican to *functional-irrelevant-files ? | 12:55 |
| rosmaita | actually, should probably do a custom string | 12:56 |
| rosmaita | i think we basically want gate-irrelevant-files, but don't want to include ^(test-|)requirements.txt$ | 12:58 |
| tosky | I guess a custom string is probably easier | 13:01 |
| tosky | that said, I'm not sure why requirements are excluded, as bumping them could affect any test | 13:01 |
| rosmaita | i think the idea is the unit and functional tests should catch incompatabilities | 13:02 |
| rosmaita | i'll put up a patch and we can discuss on the patch | 13:03 |
| tosky | rosmaita: except they don't, not for all cases, not with the mocking | 13:13 |
| opendevreview | Brian Rosmaita proposed openstack/cinder master: Make lvm-lio-barbican a canary job https://review.opendev.org/c/openstack/cinder/+/850002 | 13:18 |
| whoami-rajat | reminder: Cinder Festival of Reviews today, 1400-1600 UTC | 13:58 |
| whoami-rajat | join us in meetpad: https://meetpad.opendev.org/cinder-festival-of-reviews | 13:58 |
| whoami-rajat | or go directly to the etherpad: https://etherpad.opendev.org/p/cinder-festival-of-reviews | 13:58 |
| whoami-rajat | jungleboyj rosmaita smcginnis tosky whoami-rajat m5z e0ne geguileo eharney walshh_ jbernard sfernand enriquetaso hemna fabiooliveira | 13:58 |
| whoami-rajat | rosmaita, geguileo can you hear me on meetpad? | 14:01 |
| geguileo | no I don't hear anything... | 14:01 |
| whoami-rajat | :( | 14:01 |
| rosmaita | not now, just hear background noise | 14:01 |
| rosmaita | i thought i heard you earlier, though | 14:01 |
| geguileo | I just talked, did anyone hear me? | 14:03 |
| rosmaita | yes, we can hear you | 14:03 |
| rosmaita | but guess you can't hear us | 14:03 |
| geguileo | no, I can't hear anybody, now a bit of noise | 14:04 |
| geguileo | rejoining | 14:04 |
| whoami-rajat | https://redhat.bluejeans.com/556681290 | 14:05 |
| whoami-rajat | everyone join this link ^ jungleboyj rosmaita smcginnis tosky whoami-rajat m5z e0ne geguileo eharney walshh_ jbernard sfernand enriquetaso hemna fabiooliveira | 14:05 |
| * enriquetaso connecting | 14:09 | |
| geguileo | whoami-rajat: ok, bluejeans is failing again... | 14:12 |
| geguileo | will try with firefox | 14:12 |
| whoami-rajat | geguileo, oh, forgot you had issues with bluejeans ... | 14:12 |
| geguileo | yeah, I updated chrome, but apparently it still fails | 14:13 |
| whoami-rajat | geguileo, have you tried using the app? | 14:16 |
| geguileo | whoami-rajat: no, I haven't, but seems to work with FF | 14:16 |
| enriquetaso | #link https://etherpad.opendev.org/p/cinder-festival-of-reviews | 14:16 |
| whoami-rajat | geguileo, since you might have missed the conversation, we're going to review the XS patches today (because of the issue with nvme) http://tiny.cc/cinder-reviewfest | 14:16 |
| whoami-rajat | thanks enriquetaso | 14:16 |
| whoami-rajat | geguileo, ack, I've started using app recently and it works fine | 14:17 |
| whoami-rajat | eharney, i think you were mentioning something related to this https://review.opendev.org/c/openstack/cinder/+/849869 | 14:32 |
| opendevreview | chenwei proposed openstack/cinder master: Fix: base64.encodestring() which no longer exists in Python 3.9 https://review.opendev.org/c/openstack/cinder/+/850015 | 14:37 |
| opendevreview | Eric Harney proposed openstack/os-brick master: Move mypy job to check queue (non-voting) https://review.opendev.org/c/openstack/os-brick/+/849840 | 14:43 |
| opendevreview | Eric Harney proposed openstack/cinder master: SQLAlchemy API: Remove unused ref_get methods https://review.opendev.org/c/openstack/cinder/+/835878 | 14:46 |
| whoami-rajat | geguileo, unrelated to the XS review fest -- out of your 4 patches for brick you mentioned in cinder meeting, this one hasn't made much progress and we've to do the z-2 release today so wanted to ask if this could be skipped for current release and part of a later release? https://review.opendev.org/c/openstack/os-brick/+/836059 | 14:46 |
| whoami-rajat | actually the deadline was yesterday but I can update the release team by today EOD | 14:47 |
| opendevreview | Eric Harney proposed openstack/os-brick master: Move mypy job to check queue (non-voting) https://review.opendev.org/c/openstack/os-brick/+/849840 | 14:48 |
| geguileo | whoami-rajat: sorry, missed your ping | 15:05 |
| geguileo | whoami-rajat: yeah, that's a non-trivial patch and eharney has to go back to review it (and I may have to make changes to the patch) | 15:06 |
| geguileo | whoami-rajat: so we'll have to postpone it to the next time we release | 15:06 |
| whoami-rajat | geguileo, cool, so I will update release team when your final patch merges for the new os-brick/lock_path change | 15:07 |
| whoami-rajat | thanks | 15:07 |
| eharney | geguileo: fwiw, i do like your idea of just raising NotImplemented in the cryptsetup code much better | 15:07 |
| geguileo | whoami-rajat: thank you | 15:07 |
| geguileo | eharney: ok, I don't love it, but I don't really care all that much about that one lol | 15:08 |
| eharney | just because i don't want to unravel some effort to fix things in the cryptsetup configuration if it doesn't work as expected | 15:08 |
| geguileo | eharney: and the patch talks about LUKS, so it makes sense that I don't touch cryptsetup | 15:08 |
| geguileo | I'll update the patch now | 15:08 |
| geguileo | oh, the patch didn't talk specifically about LUKSv2! | 15:09 |
| geguileo | I'll update the commit message so it limits the scope to LUKS | 15:09 |
| *** dviroel is now known as dviroel|lunch | 15:16 | |
| opendevreview | Merged openstack/os-brick master: NVMe-oF: Remove misleading exception from logs https://review.opendev.org/c/openstack/os-brick/+/836055 | 15:20 |
| opendevreview | Merged openstack/os-brick master: Update README to drop py3.6 reference https://review.opendev.org/c/openstack/os-brick/+/849026 | 15:28 |
| opendevreview | Gorka Eguileor proposed openstack/os-brick master: LUKS: Support extending host attached volumes https://review.opendev.org/c/openstack/os-brick/+/836059 | 15:40 |
| geguileo | eharney: ^ | 15:40 |
| whoami-rajat | we're ending festival of XS reviews here. | 16:00 |
| whoami-rajat | thanks everyone for attending, have a great weekend! | 16:00 |
| opendevreview | Merged openstack/cinder master: db: Don't use legacy calling style of select() https://review.opendev.org/c/openstack/cinder/+/837354 | 16:08 |
| opendevreview | Merged openstack/cinder master: db: Don't use strings to indicate relationship names https://review.opendev.org/c/openstack/cinder/+/837355 | 16:08 |
| opendevreview | Merged openstack/cinder master: db: Remove use of 'as_scalar()' https://review.opendev.org/c/openstack/cinder/+/837356 | 16:08 |
| opendevreview | Merged openstack/python-cinderclient master: Fix extension loading from python path https://review.opendev.org/c/openstack/python-cinderclient/+/847743 | 16:12 |
| *** akekane_ is now known as ahishekk | 16:14 | |
| *** dviroel|lunch is now known as dviroel | 16:32 | |
| opendevreview | Alexander Malashenko proposed openstack/cinder master: Cinder displays the volume size provided by the driver, when creating the volume with enabled cache. https://review.opendev.org/c/openstack/cinder/+/836973 | 16:46 |
| opendevreview | Alexander Malashenko proposed openstack/cinder master: Cinder displays the volume size provided by the driver, when creating the volume with enabled cache. https://review.opendev.org/c/openstack/cinder/+/836973 | 16:50 |
| opendevreview | Merged openstack/cinder master: HPE3PAR: Correct volume name in ERROR log https://review.opendev.org/c/openstack/cinder/+/849803 | 18:07 |
| opendevreview | Merged openstack/cinder master: Update docs for powervault driver compatability https://review.opendev.org/c/openstack/cinder/+/847624 | 18:07 |
| opendevreview | Merged openstack/cinder master: PowerStore driver - Request data validation fix https://review.opendev.org/c/openstack/cinder/+/849626 | 18:24 |
| opendevreview | Merged openstack/cinder master: Bump moto version to support py3.9 https://review.opendev.org/c/openstack/cinder/+/847798 | 18:24 |
| opendevreview | Merged openstack/cinderlib master: Update doc/requirements.txt https://review.opendev.org/c/openstack/cinderlib/+/845126 | 18:24 |
| opendevreview | Simon Dodsley proposed openstack/cinder master: Add Pure Storage NVMe-RoCE driver https://review.opendev.org/c/openstack/cinder/+/799871 | 18:31 |
| opendevreview | Simon Dodsley proposed openstack/cinder master: Add Pure Storage NVMe-RoCE driver https://review.opendev.org/c/openstack/cinder/+/799871 | 18:36 |
| opendevreview | Simon Dodsley proposed openstack/cinder master: Add additional transport type constants https://review.opendev.org/c/openstack/cinder/+/849690 | 18:40 |
| opendevreview | Merged openstack/os-brick master: Support independent file lock path https://review.opendev.org/c/openstack/os-brick/+/849324 | 19:01 |
| *** tobias-urdin is now known as tobias-urdin_pto | 19:19 | |
| opendevreview | Simon Dodsley proposed openstack/cinder master: Add Pure Storage NVMe-RoCE driver https://review.opendev.org/c/openstack/cinder/+/799871 | 19:49 |
| opendevreview | Simon Dodsley proposed openstack/os-brick master: LUKS: Support extending host attached volumes https://review.opendev.org/c/openstack/os-brick/+/836059 | 19:59 |
| *** dviroel is now known as dviroel|biab | 20:09 | |
| opendevreview | Simon Dodsley proposed openstack/os-brick master: Support shared_targets tristate value https://review.opendev.org/c/openstack/os-brick/+/836063 | 21:04 |
| *** dviroel|biab is now known as dviroel | 21:08 | |
| opendevreview | Brian Rosmaita proposed openstack/cinderlib master: DNM: specify yum version in playbook https://review.opendev.org/c/openstack/cinderlib/+/849432 | 21:12 |
| opendevreview | Brian Rosmaita proposed openstack/cinderlib master: DNM: check cinderlib-ceph-functional https://review.opendev.org/c/openstack/cinderlib/+/850075 | 21:25 |
| *** dviroel is now known as dviroel|out | 21:37 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!
