Wednesday, 2024-05-08

opendevreviewYan Xiao proposed openstack/swift master: tests: Check proxy-logging stats on the wire  https://review.opendev.org/c/openstack/swift/+/89696713:59
opendevreviewYan Xiao proposed openstack/swift master: stats: API for native labeled metrics  https://review.opendev.org/c/openstack/swift/+/90988213:59
opendevreviewTim Burke proposed openstack/swift master: cors: Allow requests with credentials  https://review.opendev.org/c/openstack/swift/+/91869418:58
opendevreviewASHWIN A NAIR proposed openstack/python-swiftclient master: support part-num in python swiftclient  https://review.opendev.org/c/openstack/python-swiftclient/+/90202019:19
timburke#startmeeting swift21:00
opendevmeetMeeting started Wed May  8 21:00:05 2024 UTC and is due to finish in 60 minutes.  The chair is timburke. Information about MeetBot at http://wiki.debian.org/MeetBot.21:00
opendevmeetUseful Commands: #action #agreed #help #info #idea #link #topic #startvote.21:00
opendevmeetThe meeting name has been set to 'swift'21:00
timburkewho's here for the swift meeting?21:00
mattolivero/21:00
mattolivermight be just you and me again :) 21:00
timburkeit might :-) that's ok21:01
timburkeas usual, the agenda's at21:01
timburke#link https://wiki.openstack.org/wiki/Meetings/Swift21:01
timburkefirst up21:01
timburke#topic busted probe tests21:01
timburkelately probe tests have been reliably failing -- it seems to be a mirror issue that should resolve itself, though21:02
mattoliveroh ok, interesting21:03
timburke#link https://zuul.opendev.org/t/openstack/builds?job_name=swift-probetests-centos-9-stream21:03
timburkesame issue impacts the cors tests, since they build off the probe test job def21:04
mattoliveroh can't contact the centos-nfv-openvswitch repo21:04
mattoliveryeah, that doesn't sound like us. either the repo has issues or the nodepool image (I assume the former). 21:04
timburkei know i saw some "Downloading successful, but checksum doesn't match."21:05
timburkei recommend holding off on rechecks for now, but continuing to do normal work -- once we see that the jobs have fixed themselves, we can go around issuing rechecks21:06
mattoliveryeah, so network/repo issue21:06
mattoliverkk21:06
timburkenext up21:06
timburke#topic py312 support21:06
timburke#link https://review.opendev.org/c/openstack/swift/+/91790621:06
patch-botpatch 917906 - swift - Use ClosingMapper to ensure prompt client disconne... - 4 patch sets21:06
timburke#link https://review.opendev.org/c/openstack/swift/+/91787821:06
patch-botpatch 917878 - swift - Test under py312 - 5 patch sets21:06
jianjianokay, stop rechecks21:07
timburkeboth patches are now approved -- as soon as the probe test issue resolves, we can get 'em merged! \o/21:07
mattolivernice!21:07
timburkethanks to acoles for refactoring my hack into a more-useful ClosingMapper -- i suspect this isn't the last time we'll find ourselves wanting to propagate closes like that21:08
timburkenext up21:08
timburke#topic utils refactor21:09
timburkeit continues! as promised, the first patch in the chain landed21:09
timburkethe rebase fallout wasn't actually near so bad as i'd feared21:09
mattoliveryeah, I agree, which is nice21:09
timburkethe next patch in the chain has landed too21:09
timburke#link https://review.opendev.org/c/openstack/swift/+/91482821:09
patch-botpatch 914828 - swift - Move statsd testing to its own module (MERGED) - 9 patch sets21:09
timburkeafter that, we're looking at reworking statsd configuration21:10
timburke#link https://review.opendev.org/c/openstack/swift/+/91548321:10
patch-botpatch 915483 - swift - stats: Refactor StatsdClient config - 8 patch sets21:10
mattoliverkinda loving your thought about the `from x import ( \n<module1>\n <module2>\n)`21:10
mattoliverie a module on each line, burn a line but help with conflicts21:11
mattoliverI mean I love the idea of helping with conflicts, assuming it does. (which I think it will). Now I want a conflict to test :P 21:11
timburkei also was talking with mattoliver about it, who pointed out that the #noqa imports we add for the sake of out-of-tree consumers also meant that our in-tree callers hadn't been updated21:11
timburkeso yeah, i went and tried to update them :-)21:12
mattoliver(oh yeah I'm jumping the gun a bit :P) 21:12
timburke#link https://review.opendev.org/c/openstack/swift/+/91813921:12
patch-botpatch 918139 - swift - Un-chain imports (swift/) - 2 patch sets21:12
timburkeno worries :-)21:12
jianjiani noticed some of code within swift/common/utils/__init__.py were moved to individual file related to this effort, what's the purpose to move them to a new module, are we trying to reduce the line of code of this file?21:12
timburkejianjian, yeah, in no small part -- there used to just be the one swift/common/utils.py, and it was massive21:13
jianjianack21:14
jianjianI am keeping adding new code to that file, recently the patch of cooperative token, hope it's okay :-)21:14
timburkehttps://github.com/openstack/swift/blob/2.31.0/swift/common/utils.py had it at nearly 7k lines21:14
jianjianyeah, that's too much21:15
timburkei mean, we're only down to 5k now -- it's definitely a long-ish-term process21:16
timburkea while back clayg wrote up https://bugs.launchpad.net/swift/+bug/2015274 and we just chip away at it when we can (and it seems to make sense)21:17
patch-botBug #2015274 - utils module is too big - break it up (In Progress)21:17
jianjiangot it, i will think about another potential place for cooperative token to land, later after all testing21:17
zaitcevI think import ( x, y ) would only help if we keep the lists sorted and import enough that insertions are more than 3 lines apart.21:18
mattoliverif there is a place for something put it there, otherwise init is fine. we can always move it later :)21:18
mattoliveroh good insight zaitcev, thanks21:19
jianjiansounds good21:19
timburkeif you wanted to shift it in your patch to something like swift/common/utils/cooperative_token.py (or even swift/common/cooperative_token.py) i bet it'd be well-received. but assuming it's reasonably small, i don't think reviewers will give you too much grief for having it in utils/__init__.py21:19
mattoliver+121:19
jianjianproduction code not that much, but I remember added ~1K line to test_utils.py21:20
timburkeoh yeah, that one's at like 9k today...21:21
jianjianmaybe test file is okay 😉21:22
timburkei know part of clayg's complaint was that he had some vim integration that'd run flake8 automatically on save, and the files were getting big enough that it was a non-trivial delay21:23
timburkeanyway21:23
timburkenext up21:23
jianjianI see. thanks all for the suggestions.21:23
timburke#topic container broker row insertion order21:23
timburkei mentioned this was an issue that acoles saw on feature/mpu21:24
timburkeand now i've got a patch for master to address it!21:24
timburke#link https://review.opendev.org/c/openstack/swift/+/91814121:24
patch-botpatch 918141 - swift - container: Ensure a consistent DB insertion order - 3 patch sets21:24
timburkei only targeted the object rows for now; there's probably an argument that we should do something similar for shard ranges21:25
timburkebut this was easy enough to find tests i could tweak to exercise it :-)21:26
mattoliveroh cool, I'll check it out21:26
mattoliverseeing as I've seen this in my dev world21:27
timburkeiirc, the account backend doesn't need it, because merge_items always acts immediately (no pending file)21:27
timburkethanks mattoliver21:27
timburkenext up21:28
mattoliverI tihnk I send you that link to the shard_range insertion tests that I had the same issue. I'll try it on that too21:28
timburke#topic swiftclient stat output bug21:28
timburkethe fix landed!21:28
timburke#link https://review.opendev.org/c/openstack/python-swiftclient/+/91613521:28
patch-botpatch 916135 - python-swiftclient - Fix swiftclient output regression (MERGED) - 7 patch sets21:28
mattoliver\o/21:28
timburkeand last up21:30
timburke#topic CORS and credentials21:30
timburkewe have a user that wants to make some cross-origin requests and has been disappointed to find that we don't send a Access-Control-Allow-Credentials header21:31
timburkei'm still not exactly clear on *why* the webapp is trying to include credentials (i don't think swift would look at any of them) but it may be a thing that my user doesn't have a lot of control over21:32
timburkeso i threw together a quick patch to start sending it back21:33
timburke#link https://review.opendev.org/c/openstack/swift/+/91869421:33
patch-botpatch 918694 - swift - cors: Allow requests with credentials - 1 patch set21:33
mattoliverkk, I guess it's time to learn cors (it's one of those things that keeps slipping out of my mind) :P 21:34
mattoliver*relearn21:34
timburkei'm not *completely* sure how i want it to work, though -- should it be configurable? at the cluster level, or per-container? is it *actually* as safe to do this as i've been telling myself it is?21:34
timburkeso, if anyone else has a chance to think on it, i'd appreciate it21:35
mattoliverwill do. 21:36
timburkealso, as part of this i learned that CORS is definitely a still-evolving space -- https://www.w3.org/TR/2014/REC-cors-20140116/ (which either is, or is close to, the doc i'd been looking at the last time i thought about it) has been superseded by https://fetch.spec.whatwg.org/21:37
mattoliverI mean do the clients just look for a header response or a true value. I mean if we say False, ie we don't take creds, will it still work and they wont send them?21:38
mattoliveror will they just fail out. because we don't care about them so don't take them, so a false isn't lying :P 21:39
mattoliverbut anyway, I need to do some actaul reading on it I guess. 21:39
timburkeamong the differences, the old notion of "simple headers" did not include Range (so a range request would require a preflight request), while the new "safelisted request-headers" *does* include Range -- so i'm not sure the CORS tests i wrote are still exercising all the paths we want21:39
timburkepretty sure sending false will prevent the caller from seeing the response at all, same as not sending the header21:40
timburkethat's all i've got21:41
timburke#topic open discussion21:41
mattoliverkk, just checking :) 21:41
mattoliverreading now :) 21:41
timburkeanything else we should bring up this week?21:41
timburkeoh! mattoliver, i went looking for policy-migration-related patches... and found p 173580 and p 20932921:43
patch-bothttps://review.opendev.org/c/openstack/swift/+/173580 - swift - wip: Cluster assisted Storage Policy Migration - 14 patch sets21:43
patch-bothttps://review.opendev.org/c/openstack/swift/+/209329 - swift - WIP: Changing Policies - 18 patch sets21:43
timburkethe first one, i saw you'd rebased remarkably recently, given the ancient patch number21:44
timburkeand it was on top of p 800748 -- which had *completely* fallen off my radar21:44
patch-bothttps://review.opendev.org/c/openstack/swift/+/800748 - swift - sharder: update shard storage_policy_index if root... - 20 patch sets21:44
mattoliveroh nice, I'll make a note of them. Could be useful if we want to extend the scope of the ops tool project.21:45
mattoliveroh yeah. I think I rebased one when it looked like something we could blow the dust off at somepoint. but obvously it never went anywhere :P 21:45
timburkeshould we still try to address that? i don't remember how we came upon the mis-matched policy issue, but i feel like we *did* see some impacts in prod...21:46
mattoliveryeah, we probably should. 21:47
timburkei think at least one of those patches would give a pretty good starting point for the ops tool: https://review.opendev.org/c/openstack/swift/+/173580/14/swift/cli/container_migrate.py21:47
patch-botpatch 173580 - swift - wip: Cluster assisted Storage Policy Migration - 14 patch sets21:47
mattoliverI think I wrote a patch that landed that stopped it being as big an issue (ie a shard will return whatever rows the stroage policy you ask it) 21:48
timburkeoh nice, that's right21:48
mattoliverBut yeah we need to migrate the s-p so it can match the roots (as that would be the most expected behaviour) 21:49
timburkethere it is! took me a bit to find p 80342321:51
patch-bothttps://review.opendev.org/c/openstack/swift/+/803423 - swift - container-server: return objects of a given policy (MERGED) - 10 patch sets21:51
mattoliveroh nice, your gerrit archeology is always impressive :) 21:52
timburkeall right, i think i'll call it21:52
timburkethank you all for coming, and thank you for working on swift!21:52
timburke#endmeeting21:52
opendevmeetMeeting ended Wed May  8 21:52:46 2024 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)21:52
opendevmeetMinutes:        https://meetings.opendev.org/meetings/swift/2024/swift.2024-05-08-21.00.html21:52
opendevmeetMinutes (text): https://meetings.opendev.org/meetings/swift/2024/swift.2024-05-08-21.00.txt21:52
opendevmeetLog:            https://meetings.opendev.org/meetings/swift/2024/swift.2024-05-08-21.00.log.html21:52
opendevreviewShreeya Deshpande proposed openstack/swift master: stats: Refactor StatsdClient config  https://review.opendev.org/c/openstack/swift/+/91548322:09
opendevreviewTim Burke proposed openstack/python-swiftclient master: Add reset function for ReadableToIterable  https://review.opendev.org/c/openstack/python-swiftclient/+/91870122:29
opendevreviewClay Gerrard proposed openstack/python-swiftclient master: wip: failing test for SwiftService.upload_object with BytesIO  https://review.opendev.org/c/openstack/python-swiftclient/+/91870323:52

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!