[10:24:40] lunch [12:31:26] merged Erik's patch for the cirrus opensearch 3.8 image, added a tag to trigger a build and publish to the registry [12:58:13] should be up at: docker-registry.wm.o/repos/search-platform/cirrussearch-opensearch-image:v3.8.0-0 [13:20:00] \o [13:20:02] thanks! [13:32:59] o/ [14:13:31] o/ I have a conflict with the retro today: there’s a people-manager workshop on the ongoing US unionization process that won’t be recorded, and given that we have US-based colleagues I’d like to attend. [14:13:31] Since it’s also been a while since our last retro, I don’t want to just skip this one. I’m inclined to reschedule it rather than cancel. If you feel there isn’t much we need to reflect on right now, though, let me know and we can reconsider. [14:15:39] pfischer: fine by me to re-schedule [14:17:01] sigh... back into the maze of SpecialSearch/PrefixSearch/CirrusSearch, still pondering how to best allow an extension to tune prefix search behavior on special pages... [14:20:56] i'm just lost on a naming problem...i have QueryClassVoter, NamespaceVoter, QueryLengthVoter. The problem is they return an enum of either VETO (end checking), FORCE (end checking), CANDIDATE (add score to list of routes that accept), and REJECT (do nothing)....but the voters are all just called voters [14:21:34] i guess i mean it's not obvious in config that NamespaceVoter just VETO's things in the wrong namespaces, but QueryLengthVoter will give a route candidate status if it's long enough [14:22:59] a FORCE one implies some ordering? [14:23:00] i guess it's almost like there are two stages of pre-checks then candidacy checks, but i kinda rolled it all together. but it already feels a bit over-engineered, not sure i want to add more separation [14:23:29] dcausse: FORCE would be for SemanticDebugOptionVoter, that checks debug options and just says do it regardless of other things (unless a VETO came earlier in the sequential list) [14:24:16] basically we run the voters in the order they were provided, the first VETO or FORCE ends checking and just "wins" [14:24:26] but candidate and reject are kind the "normal" path [14:25:20] yes but I suspect we still don't want "force" if it's not on the required ns, so the ns check has to happen before the cirrus debug force check [14:25:58] dcausse: yea that depends on config ordering, so semantic would be ordered with namespace and query class VETO's first, then the force for debug options, then the query length check which can make it a candidate [14:28:21] is the dispatcher seeing a list of voters to call and making the decisions or is it still happening in the route itself? [14:29:51] I guess it must happen in the dispatch logic to enforce a "FORCE"? [14:29:53] dcausse: this is all moved down to the dispatcher, the dispatcher gets a `VotedSearchQueryRoute` which implements the existing SearchQueryRoute but delegates to the voters [14:30:18] so we now have a VotedSearchQueryRoute::decide which returns a RouteDecision, that decision says force/reject/etc [14:30:48] the VotedSeachQueryRoute has a list of voters, it runs them in order aggregating a decision from the voters [14:31:11] and by default it's just CANDIDATE to re-use existing scores from existing routes? [14:31:16] i suppose maybe it would be easier if i put the code up for review, it's mostly there i'm just not completely happy with it [14:31:41] sure, will certainly be easier to make sure I talk about the same thing :) [14:31:43] dcausse: yea if it votes CANDIDATE then it's basically the pre-existing system, sort routes by score and choose best [14:32:05] i suppose this adds a configurable layer of accept/reject on top of that [14:32:13] I see [14:32:31] meh, i went to finish my rebase and i have conflicts :P i'll put it up today but will take a min [14:32:39] np! [14:37:01] but if two routes have a FORCE we'll still rely on the search profile registration order I guess? probably not a problem at this point and could be solved by giving a "priority" so that the "foreach( $routes )" runs in a "pre-defined" order [14:51:36] Can't make retro today but ping me if you need anything [16:24:55] lol, i love that for "how do birds navigate during migration?" the second passage highlights "using a variety of methods" [16:25:51] :) [16:26:44] test harness failed to run on cindy for v3.8.0-0...guess i actually have to run it locally and look. Was mildly hopeful it would just work [16:27:06] dcausse: primary patch for the dispatching is: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CirrusSearch/+/1342743 [16:27:18] ebernhardson: it's only the version check apparently, so expected :) [16:27:24] looking [16:27:53] oh, yea that makes sense [16:32:26] i'm a little annoyed at how much code this turned into, but lots of it is boilerplate and relatively simple [16:34:21] yes... making something generic and driven by config always results in quite a lot of boiler plate, see all that search profiles thingy :/ [16:42:31] ok, for customizing completion on special pages I think I'll make cirrus own some of the logic and make the mw-core logic available via a public method, introducing yet another hook sounds painful for this... [17:27:38] looks like cindy passed on 3.x :) [17:28:09] took forever but possibly that's because on the patch chain for query routing [17:28:21] dinner [17:58:53] yea i think it was the 5 patches for routing that delayed it, they were slowly coming in