opendevreview | Yan Xiao proposed openstack/swift master: tests: Check proxy-logging stats on the wire https://review.opendev.org/c/openstack/swift/+/896967 | 13:59 |
---|---|---|
opendevreview | Yan Xiao proposed openstack/swift master: stats: API for native labeled metrics https://review.opendev.org/c/openstack/swift/+/909882 | 13:59 |
opendevreview | Tim Burke proposed openstack/swift master: cors: Allow requests with credentials https://review.opendev.org/c/openstack/swift/+/918694 | 18:58 |
opendevreview | ASHWIN A NAIR proposed openstack/python-swiftclient master: support part-num in python swiftclient https://review.opendev.org/c/openstack/python-swiftclient/+/902020 | 19:19 |
timburke | #startmeeting swift | 21:00 |
opendevmeet | Meeting 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 |
opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 21:00 |
opendevmeet | The meeting name has been set to 'swift' | 21:00 |
timburke | who's here for the swift meeting? | 21:00 |
mattoliver | o/ | 21:00 |
mattoliver | might be just you and me again :) | 21:00 |
timburke | it might :-) that's ok | 21:01 |
timburke | as usual, the agenda's at | 21:01 |
timburke | #link https://wiki.openstack.org/wiki/Meetings/Swift | 21:01 |
timburke | first up | 21:01 |
timburke | #topic busted probe tests | 21:01 |
timburke | lately probe tests have been reliably failing -- it seems to be a mirror issue that should resolve itself, though | 21:02 |
mattoliver | oh ok, interesting | 21:03 |
timburke | #link https://zuul.opendev.org/t/openstack/builds?job_name=swift-probetests-centos-9-stream | 21:03 |
timburke | same issue impacts the cors tests, since they build off the probe test job def | 21:04 |
mattoliver | oh can't contact the centos-nfv-openvswitch repo | 21:04 |
mattoliver | yeah, that doesn't sound like us. either the repo has issues or the nodepool image (I assume the former). | 21:04 |
timburke | i know i saw some "Downloading successful, but checksum doesn't match." | 21:05 |
timburke | i 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 rechecks | 21:06 |
mattoliver | yeah, so network/repo issue | 21:06 |
mattoliver | kk | 21:06 |
timburke | next up | 21:06 |
timburke | #topic py312 support | 21:06 |
timburke | #link https://review.opendev.org/c/openstack/swift/+/917906 | 21:06 |
patch-bot | patch 917906 - swift - Use ClosingMapper to ensure prompt client disconne... - 4 patch sets | 21:06 |
timburke | #link https://review.opendev.org/c/openstack/swift/+/917878 | 21:06 |
patch-bot | patch 917878 - swift - Test under py312 - 5 patch sets | 21:06 |
jianjian | okay, stop rechecks | 21:07 |
timburke | both patches are now approved -- as soon as the probe test issue resolves, we can get 'em merged! \o/ | 21:07 |
mattoliver | nice! | 21:07 |
timburke | thanks 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 that | 21:08 |
timburke | next up | 21:08 |
timburke | #topic utils refactor | 21:09 |
timburke | it continues! as promised, the first patch in the chain landed | 21:09 |
timburke | the rebase fallout wasn't actually near so bad as i'd feared | 21:09 |
mattoliver | yeah, I agree, which is nice | 21:09 |
timburke | the next patch in the chain has landed too | 21:09 |
timburke | #link https://review.opendev.org/c/openstack/swift/+/914828 | 21:09 |
patch-bot | patch 914828 - swift - Move statsd testing to its own module (MERGED) - 9 patch sets | 21:09 |
timburke | after that, we're looking at reworking statsd configuration | 21:10 |
timburke | #link https://review.opendev.org/c/openstack/swift/+/915483 | 21:10 |
patch-bot | patch 915483 - swift - stats: Refactor StatsdClient config - 8 patch sets | 21:10 |
mattoliver | kinda loving your thought about the `from x import ( \n<module1>\n <module2>\n)` | 21:10 |
mattoliver | ie a module on each line, burn a line but help with conflicts | 21:11 |
mattoliver | I 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 |
timburke | i 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 updated | 21:11 |
timburke | so 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/+/918139 | 21:12 |
patch-bot | patch 918139 - swift - Un-chain imports (swift/) - 2 patch sets | 21:12 |
timburke | no worries :-) | 21:12 |
jianjian | i 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 |
timburke | jianjian, yeah, in no small part -- there used to just be the one swift/common/utils.py, and it was massive | 21:13 |
jianjian | ack | 21:14 |
jianjian | I am keeping adding new code to that file, recently the patch of cooperative token, hope it's okay :-) | 21:14 |
timburke | https://github.com/openstack/swift/blob/2.31.0/swift/common/utils.py had it at nearly 7k lines | 21:14 |
jianjian | yeah, that's too much | 21:15 |
timburke | i mean, we're only down to 5k now -- it's definitely a long-ish-term process | 21:16 |
timburke | a 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-bot | Bug #2015274 - utils module is too big - break it up (In Progress) | 21:17 |
jianjian | got it, i will think about another potential place for cooperative token to land, later after all testing | 21:17 |
zaitcev | I 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 |
mattoliver | if there is a place for something put it there, otherwise init is fine. we can always move it later :) | 21:18 |
mattoliver | oh good insight zaitcev, thanks | 21:19 |
jianjian | sounds good | 21:19 |
timburke | if 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__.py | 21:19 |
mattoliver | +1 | 21:19 |
jianjian | production code not that much, but I remember added ~1K line to test_utils.py | 21:20 |
timburke | oh yeah, that one's at like 9k today... | 21:21 |
jianjian | maybe test file is okay 😉 | 21:22 |
timburke | i 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 delay | 21:23 |
timburke | anyway | 21:23 |
timburke | next up | 21:23 |
jianjian | I see. thanks all for the suggestions. | 21:23 |
timburke | #topic container broker row insertion order | 21:23 |
timburke | i mentioned this was an issue that acoles saw on feature/mpu | 21:24 |
timburke | and now i've got a patch for master to address it! | 21:24 |
timburke | #link https://review.opendev.org/c/openstack/swift/+/918141 | 21:24 |
patch-bot | patch 918141 - swift - container: Ensure a consistent DB insertion order - 3 patch sets | 21:24 |
timburke | i only targeted the object rows for now; there's probably an argument that we should do something similar for shard ranges | 21:25 |
timburke | but this was easy enough to find tests i could tweak to exercise it :-) | 21:26 |
mattoliver | oh cool, I'll check it out | 21:26 |
mattoliver | seeing as I've seen this in my dev world | 21:27 |
timburke | iirc, the account backend doesn't need it, because merge_items always acts immediately (no pending file) | 21:27 |
timburke | thanks mattoliver | 21:27 |
timburke | next up | 21:28 |
mattoliver | I tihnk I send you that link to the shard_range insertion tests that I had the same issue. I'll try it on that too | 21:28 |
timburke | #topic swiftclient stat output bug | 21:28 |
timburke | the fix landed! | 21:28 |
timburke | #link https://review.opendev.org/c/openstack/python-swiftclient/+/916135 | 21:28 |
patch-bot | patch 916135 - python-swiftclient - Fix swiftclient output regression (MERGED) - 7 patch sets | 21:28 |
mattoliver | \o/ | 21:28 |
timburke | and last up | 21:30 |
timburke | #topic CORS and credentials | 21:30 |
timburke | we 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 header | 21:31 |
timburke | i'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 over | 21:32 |
timburke | so i threw together a quick patch to start sending it back | 21:33 |
timburke | #link https://review.opendev.org/c/openstack/swift/+/918694 | 21:33 |
patch-bot | patch 918694 - swift - cors: Allow requests with credentials - 1 patch set | 21:33 |
mattoliver | kk, 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 | *relearn | 21:34 |
timburke | i'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 |
timburke | so, if anyone else has a chance to think on it, i'd appreciate it | 21:35 |
mattoliver | will do. | 21:36 |
timburke | also, 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 |
mattoliver | I 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 |
mattoliver | or 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 |
mattoliver | but anyway, I need to do some actaul reading on it I guess. | 21:39 |
timburke | among 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 want | 21:39 |
timburke | pretty sure sending false will prevent the caller from seeing the response at all, same as not sending the header | 21:40 |
timburke | that's all i've got | 21:41 |
timburke | #topic open discussion | 21:41 |
mattoliver | kk, just checking :) | 21:41 |
mattoliver | reading now :) | 21:41 |
timburke | anything else we should bring up this week? | 21:41 |
timburke | oh! mattoliver, i went looking for policy-migration-related patches... and found p 173580 and p 209329 | 21:43 |
patch-bot | https://review.opendev.org/c/openstack/swift/+/173580 - swift - wip: Cluster assisted Storage Policy Migration - 14 patch sets | 21:43 |
patch-bot | https://review.opendev.org/c/openstack/swift/+/209329 - swift - WIP: Changing Policies - 18 patch sets | 21:43 |
timburke | the first one, i saw you'd rebased remarkably recently, given the ancient patch number | 21:44 |
timburke | and it was on top of p 800748 -- which had *completely* fallen off my radar | 21:44 |
patch-bot | https://review.opendev.org/c/openstack/swift/+/800748 - swift - sharder: update shard storage_policy_index if root... - 20 patch sets | 21:44 |
mattoliver | oh 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 |
mattoliver | oh 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 |
timburke | should 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 |
mattoliver | yeah, we probably should. | 21:47 |
timburke | i 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.py | 21:47 |
patch-bot | patch 173580 - swift - wip: Cluster assisted Storage Policy Migration - 14 patch sets | 21:47 |
mattoliver | I 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 |
timburke | oh nice, that's right | 21:48 |
mattoliver | But yeah we need to migrate the s-p so it can match the roots (as that would be the most expected behaviour) | 21:49 |
timburke | there it is! took me a bit to find p 803423 | 21:51 |
patch-bot | https://review.opendev.org/c/openstack/swift/+/803423 - swift - container-server: return objects of a given policy (MERGED) - 10 patch sets | 21:51 |
mattoliver | oh nice, your gerrit archeology is always impressive :) | 21:52 |
timburke | all right, i think i'll call it | 21:52 |
timburke | thank you all for coming, and thank you for working on swift! | 21:52 |
timburke | #endmeeting | 21:52 |
opendevmeet | Meeting ended Wed May 8 21:52:46 2024 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 21:52 |
opendevmeet | Minutes: https://meetings.opendev.org/meetings/swift/2024/swift.2024-05-08-21.00.html | 21:52 |
opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/swift/2024/swift.2024-05-08-21.00.txt | 21:52 |
opendevmeet | Log: https://meetings.opendev.org/meetings/swift/2024/swift.2024-05-08-21.00.log.html | 21:52 |
opendevreview | Shreeya Deshpande proposed openstack/swift master: stats: Refactor StatsdClient config https://review.opendev.org/c/openstack/swift/+/915483 | 22:09 |
opendevreview | Tim Burke proposed openstack/python-swiftclient master: Add reset function for ReadableToIterable https://review.opendev.org/c/openstack/python-swiftclient/+/918701 | 22:29 |
opendevreview | Clay Gerrard proposed openstack/python-swiftclient master: wip: failing test for SwiftService.upload_object with BytesIO https://review.opendev.org/c/openstack/python-swiftclient/+/918703 | 23:52 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!