Page MenuHomePhabricator

Support for dynamically enabling new wikis
Closed, ResolvedPublic


Right now, given that we've made parsoid config read only, it is not possible to dynamically enable/disable wikis by adding / removing interwiki entries in the config. However, large wiki farms like wikia (and some others) might need such a feature.

Something we should look into at some point.

Event Timeline

ssastry raised the priority of this task from to Medium.
ssastry updated the task description. (Show Details)
ssastry added a project: Parsoid.
ssastry added a subscriber: ssastry.

We could probably add a hook to call a user function (defined in localsettings.js) if the provided "domain" string (which is completely arbitrary, could be URL-encoded whatever) is not present in our configuration map. The user function could return the appropriate MW api info for that domain (or null if the domain is not valid). The user function *could* do this by URL-decoding the domain string, or by doing whatever else it thinks reasonable.

Note that this would solve the "make a new wiki" problem, *not* the "change the configuration of an existing wiki" problem. For that latter, you need some explicit way to force cache invalidation/etc -- probably easier just to restart Parsoid in that case, which ensures that we're not retaining out-of-date config info.

Change 463979 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/services/parsoid@master] WIP: Allow users to dynamically configure new wikis

See also for an older approach to this issue, for a post on mediawiki-l apparently requesting this functionality, and for a post on our "support forums" (news to me that we had these, not surprising it never got seen).

Also is the way that WMF manages its own wikis.

Parsoid should probably accept a SIGHUP signal to trigger reloading config files from disk, if it doesn't already. That way the config can be updated (e.g. via puppet) and the existing Parsoid process can keep serving requests.
Right now parsoid is actually part of the new wiki creation process because it keeps it's own copy of sitematrix stashed away in its own repository:

@Krenair SIGKILL is more reliable and would cost about the same as SIGHUP. Adding SIGHUP support would probably involve a lot of tedious tracking down various bits of cached data. Besides, much of this configuration management code is going away with the upcoming Parsoid/PHP port & integration with core.

Agreed that we should be dynamically loading SiteMatrix instead of keeping a local copy. I don't know why we aren't doing that, given that we already have tools/fetch-wmf-sitematrix.js written to do the API request needed. Presumably it was to avoid an async call in the initial configuration phase, but we should be able to refactor around that w/o too much pain.

I've looked into this a little further. Based on the initial patch, I found a solution that works with the following setup:

exports.setup = function(parsoidConfig) {
  parsoidConfig.dynamicConfig = function(domain) {
      uri: 'https://' + domain + '/api.php',
      domain: domain

I amended the commit [1]. I hope this is okay. If not I want to apologize and will undo my changes immediately.

Unfortunately I can not tell if those changes are "allowed", so I'd need someone of the core developers to check this.


Thanks for the patch, looks good! And I'm fine with amending the original patch. I made a few minor suggestions. Thanks for testing this out in practice!

@cscott FYI: your latest changes on the patchset (introduction of ParsoidConfig.getPrefixFor) do not work for me. When I run node bin/server.js the constructor of ParsoidConfig get's called four times. So there are four different instances of this class used. Then, when my MediaWiki/VisualEditor client calls lib/api/routes.js invokes ParsoidConfig.getPrefixFor (and therefore implicitly ParsoidConfig.dynamicConfig). It adds the domain and the dynamic prefix to the res.locals.envOptions. When the execution then arrives at MWParserEnvironment.getParserEnv, the fields options.prefix and options.domain are properly set, so parsoidConfig.getPrefixFor(options.domain); is not being called anymore. Unfortunately the parsoidConfig object in this context is not the same as in lib/api/routes.js. Therefore the call parsoidConfig.mwApiMap.has(options.prefix) will return false and an error will be thrown.

I have added a second condition to the code in MWParserEnvironment.getParserEnv [1] to circumvent the issue described above. Now it works for me :)


@Osnard Could you see if my updated patchset works for you? Rather than explicitly test reverseMwApiMap again, I just unconditionally called ParsoidConfig#getPrefixFor() to set the prefix (even if it was already set). That's a little more consistent with the direction we really want to go (T206764: Remove `prefix` from Parsoid and use `domain` consistently as configuration key).

I can confirm that your change is working with my configuration. Thanks @cscott ! Hopefully this can be merged soon.

Change 463979 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Allow users to dynamically configure new wikis

Arlolra assigned this task to cscott.