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