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

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

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

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 subscribed.

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 :(

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

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

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

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

Kizule added subscribers: Umherirrender, Kizule.

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

Kizule removed a project: User-Kizule.
Kizule unsubscribed.

Sorry, but I can not.

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

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 :)

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 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

  NODES
chat 1
Idea 1
idea 1
Note 4
Project 11
USERS 1