Page MenuHomePhabricator

Convert CirrusSearch to use extension registration
Closed, ResolvedPublic

Description

The CirrusSearch extension needs to be converted to use the new extension registration system. More details are available on T87875.

The missing parts appear to be:

  • Profile definitions: $wgCirrusSearchCompletionProfiles & friends. These are global presumably because we want to make them configurable. Potential solution: load them in special handler? Create API to manipulate them from local configs? Default profiles are no longer loaded by CirrusSearch.php. $wgCirrusSearchCompletionProfiles & friends are now by default empty and can be populated by users when they want to customize search profiles.
  • Non-simple values: values that are either function calls or references to other values. E.g. $wgCirrusSearchIndexBaseName = wfWikiID();. Potential solution: set them in handler/hook? The problem is distinguishing between default and override. Another solution: default to null and substitute the real value later, on-demand. For some, like $wgCirrusSearchCompletionGeoContextSettings, we can change the profile value to a profile name.
  • Calculated constant values, e.g. $wgCirrusSearchDropDelayedJobsAfter = 60 * 60 * 24 * 2; // 2 days. The main issue is documenting the value and making it clear. Potential solution: separate documentation and possibly @note in the config.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Liuxinyu970226 set Security to None.
Restricted Application added a project: Discovery. · View Herald TranscriptAug 26 2015, 1:02 AM

Change 304111 had a related patch set uploaded (by Reedy):
Convert to extension registration

https://gerrit.wikimedia.org/r/304111

Restricted Application added a project: Discovery-Search. · View Herald TranscriptAug 10 2016, 9:00 PM
debt triaged this task as Low priority.Aug 11 2016, 4:59 PM
debt moved this task from needs triage to Up Next on the Discovery-Search board.
debt added a subscriber: debt.

moving to 'this quarter' for now, when we have more time to do this large amount of work. We'll need chat about what exactly needs to be done.

BTW - the patch failed in Jenkins :(

debt moved this task from Up Next to This Quarter on the Discovery-Search board.Sep 12 2016, 4:07 PM

Moving to this quarter for now...this is pretty complex

Smalyshev updated the task description. (Show Details)Dec 6 2016, 2:49 AM

Some comments on the roadblocks that @Smalyshev added into the description would be welcome from @EBernhardson, @dcausse, and @TJones.

TJones added a comment.Dec 7 2016, 2:39 PM

Took a look. Nothing to add or ask, mostly because this is far outside my area of expertise.

Per @tstarling at T140852#3139266 can we get this bumped up the priority wise?

Granted, this should come after you've finished your ES upgrades for sure

Reedy added a comment.Mar 29 2017, 2:48 PM

Lack of support to specifying class names - e.g. like $wgCirrusSearchFieldTypeOverrides and $wgCirrusSearchFieldTypes use php\class\name::class. Potential solution: so far I can see only converting back to strings. Not ideal, but we have many other contexts where class names are configured as strings.

This is what we do for auth stuff elsewhere where we want to use class names, e.g https://github.com/wikimedia/mediawiki-extensions-OATHAuth/blob/master/extension.json#L31

Change 304111 abandoned by Reedy:
Convert to extension registration

https://gerrit.wikimedia.org/r/304111

quick question: could the problem mentioned in T142652 be related to T184837?

@Oetterer I don't think this is related, this task is just to track progress on making the extension CirrusSearch compatible with the new extension registration process. It is just listing the pieces of code that make this refactoring problematic not actual problems regarding Config factories.
Perhaps we'll end up having the same issues but I don't think we have code that need to be run just after the extension is loaded.

Change 404502 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/extensions/CirrusSearch@master] Move wfWikiId resolution for CirrusSearchIndexBaseName into SearchConfig

https://gerrit.wikimedia.org/r/404502

Change 404502 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Move wfWikiId resolution for CirrusSearchIndexBaseName into SearchConfig

https://gerrit.wikimedia.org/r/404502

I will try to do this to https://gerrit.wikimedia.org/r/#/c/406854/ can be restored and merged

Restricted Application added a project: User-Zoranzoki21. · View Herald TranscriptJan 31 2018, 3:30 PM
Zoranzoki21 removed Zoranzoki21 as the assignee of this task.Jan 31 2018, 3:41 PM
Zoranzoki21 removed a project: User-Zoranzoki21.
Zoranzoki21 removed a subscriber: Zoranzoki21.

Sorry, but I can not.

dcausse updated the task description. (Show Details)Aug 21 2018, 7:36 AM
Smalyshev updated the task description. (Show Details)Apr 18 2019, 9:55 PM

Change 510815 had a related patch set uploaded (by Smalyshev; owner: Smalyshev):
[mediawiki/extensions/CirrusSearch@master] Convert CirrusSearch to extension.json

https://gerrit.wikimedia.org/r/510815

Change 512076 had a related patch set uploaded (by Smalyshev; owner: Smalyshev):
[mediawiki/extensions/CirrusSearch@master] Split multi-class files into separate files

https://gerrit.wikimedia.org/r/512076

Change 512076 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Split multi-class files into separate files

https://gerrit.wikimedia.org/r/512076

Change 510815 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Convert CirrusSearch to extension.json

https://gerrit.wikimedia.org/r/510815

Change 513068 had a related patch set uploaded (by DCausse; owner: DCausse):
[operations/mediawiki-config@master] [cirrus] Load cirrus using wfLoadExtension

https://gerrit.wikimedia.org/r/513068

dcausse updated the task description. (Show Details)May 29 2019, 9:46 AM
Reedy added a comment.May 29 2019, 9:58 AM

I know it's somewhat arbitrary, but might be a good time to bump the version of the extension in extension.json to something greater than 0.2 :)

Reedy awarded a token.May 29 2019, 9:58 AM

yes, we need to find a good way to version cirrus indeed :)
I was thinking of aligning it to the version of elasticsearch it supports. We should be at something like 6.x.

yes, we need to find a good way to version cirrus indeed :)
I was thinking of aligning it to the version of elasticsearch it supports. We should be at something like 6.x.

Seems reasonable to me. Should do the Elastica extension in the same way at the same time too

Easy to document the compat with ES too then for both :)

Change 513556 had a related patch set uploaded (by DCausse; owner: DCausse):
[operations/mediawiki-config@master] [cirrus] extension registration: don't assume default vars are set

https://gerrit.wikimedia.org/r/513556

Change 513605 had a related patch set uploaded (by DCausse; owner: DCausse):
[operations/mediawiki-config@master] [cirrus] drop most wmgCirrusSearch* ephemeral config vars [1/3]

https://gerrit.wikimedia.org/r/513605

Change 513606 had a related patch set uploaded (by DCausse; owner: DCausse):
[operations/mediawiki-config@master] [cirrus] drop most wmgCirrusSearch* ephemeral config vars [2/3]

https://gerrit.wikimedia.org/r/513606

Change 513607 had a related patch set uploaded (by DCausse; owner: DCausse):
[operations/mediawiki-config@master] [cirrus] drop most wmgCirrusSearch* ephemeral config vars [3/3]

https://gerrit.wikimedia.org/r/513607

Change 513556 merged by jenkins-bot:
[operations/mediawiki-config@master] [cirrus] extension registration: don't assume default vars are set

https://gerrit.wikimedia.org/r/513556

Change 514994 had a related patch set uploaded (by Smalyshev; owner: Smalyshev):
[operations/mediawiki-config@master] Migrate CirrusSearch to extension.json officially

https://gerrit.wikimedia.org/r/514994

Change 513068 merged by jenkins-bot:
[operations/mediawiki-config@master] [cirrus] Load cirrus using wfLoadExtension

https://gerrit.wikimedia.org/r/513068

Change 513605 merged by jenkins-bot:
[operations/mediawiki-config@master] [cirrus] drop most wmgCirrusSearch* ephemeral config vars [1/3]

https://gerrit.wikimedia.org/r/513605

Change 513606 merged by jenkins-bot:
[operations/mediawiki-config@master] [cirrus] drop most wmgCirrusSearch* ephemeral config vars [2/3]

https://gerrit.wikimedia.org/r/513606

Change 513607 merged by jenkins-bot:
[operations/mediawiki-config@master] [cirrus] drop most wmgCirrusSearch* ephemeral config vars [3/3]

https://gerrit.wikimedia.org/r/513607

Reedy closed this task as Resolved.Jun 18 2019, 1:00 PM
Reedy assigned this task to Smalyshev.

Closing as resolved, might be a few residual issues, but most of this work should be done now

Thanks! :D

Change 514994 abandoned by Smalyshev:
Migrate CirrusSearch to extension.json officially

Reason:
this seems to be already done in d6e49dee32a3a952d05b195e8686c22be17fcf33

https://gerrit.wikimedia.org/r/514994