Page MenuHomePhabricator

Create a hook to manipulate configuration on startup.
Open, MediumPublic

Description

Sometimes there is a need to dynamically change configuration defaults, based on other configuration. We see this kind of behavior in some extensions as well as the dynamic config of the WMF cluster. Traditionally, this has been done in an "extension function" callback, but these callbacks are called only at the end of Setup.php, which may be too late: the service instance that needs the configuration may already have been instantiated (see T275334 and T184837).

To resolve this issue, we hook specifically designed for manipulating configuration objects should be introduced.

Event Timeline

Change 703011 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Introduce MediaWikiConfigHook

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

I will -2 the patch, and plan to close this as declined once discussion has resolved, as what's proposed here is the exact opposite direction we have been going in for years, to make configuration static and *not* dynamic. Configuration should not be changed at runtime outside of dedicated hooks. For example, at T275334#6923618 I proposed how specifically tailored configuration settings allow for static configuration and still accomplish the use case. $wgExtensionFunctions has always been a hack and a code smell and shouldn't be replaced, it should just go away.

I will -2 the patch, and plan to close this as declined once discussion has resolved, as what's proposed here is the exact opposite direction we have been going in for years, to make configuration static and *not* dynamic. Configuration should not be changed at runtime outside of dedicated hooks. For example, at T275334#6923618 I proposed how specifically tailored configuration settings allow for static configuration and still accomplish the use case. $wgExtensionFunctions has always been a hack and a code smell and shouldn't be replaced, it should just go away.

Can you describe how we would use your approach to cover ad-hoc config manipulation like we do it in wmf-config/CommonSettings.php? Note that we need this soon, since the more code is moved to services and has config injected, the less we can use ExtensionFunctions, and the more urgent it is to have a replacement that covers this need. We need a clear migration path, and not just for us. As far as I know, this kind of ad-hoc override is a common pattern in wiki farms.

I agree that configuration should not change once MediaWiki is been initialized. But during bootstrap, we routinely manipulate configuration settings and via ExtensionRegistry. This is currently done via global variables. Why not have a mechanism that allows this to be done programmatically? Any why not move away from using global variables to do it?

Can you describe how we would use your approach to cover ad-hoc config manipulation like we do it in wmf-config/CommonSettings.php?

Here's a quick analysis of all the $wgExensionFunctions in operations/mediawiki-config https://codesearch.wmcloud.org/operations/?q=%5C%24wgExtensionFunctions&i=nope&files=&excludeFiles=&repos=

  • XFF log - move to MediaWiki initialization, generally useful outside of Wikimedia anyways
  • $wgMathFullRestbaseURL, which depends on $wgServerName (labeled as a HACK). The configuration setting could support relative URLs that begin with / and have $wgServerName prefixed at runtime. But I suspect this isn't even needed, how are other Restbase services configured? Why do none of them need this special handling? Probably Math is doing something nonstandard here, but I'd need to look into it more.
  • push-subscription-manager only on Meta-Wiki: the group shouldn't be defined in Echo, it's a non-standard group. it should only be created in wmf-config so it would only be in Meta in the first place (we did this to some other extensions, I don't remember which ones offhand). In T275334#6923618 I suggested $wgDisabledGroups = ['whatever' => true], which I think is still useful for other cases.
  • Disable CSP if no session: I think this is a temporary thing since we want to be able to quickly revert CSP and not have it cached in Varnish. Moving this to runtime, either with a $wgEnableCSPForLoggedOut or a hook to control CSP. I'd be surprised if there wasn't already an OutputPage hook that allows for disabling the CSP headers at runtime
  • Rename suppressors to oversighters group: just finish renaming the group, or support inheriting group permissions at runtime (T275334#6923618).
  • Throttle exceptions: T27000: Deploy ThrottleOverride extension to Wikimedia wikis

Note that we need this soon, since the more code is moved to services and has config injected, the less we can use ExtensionFunctions, and the more urgent it is to have a replacement that covers this need. We need a clear migration path, and not just for us. As far as I know, this kind of ad-hoc override is a common pattern in wiki farms.

Depending on when you need it, there's also extension.json callbacks, SetupAfterCache, and one more I forget off the top of my head.

Using $wgExtensionFunctions is a hack (c.f. https://www.mediawiki.org/w/index.php?diff=2421384&oldid=2311811&title=Manual%3A%24wgExtensionFunctions&type=revision) and it's probably good that these hacks are finally breaking, at it forces us to have proper solutions for all of them, which I think is the goal of all this refactoring/cleanup in the first place.

I agree that configuration should not change once MediaWiki is been initialized. But during bootstrap, we routinely manipulate configuration settings and via ExtensionRegistry. This is currently done via global variables. Why not have a mechanism that allows this to be done programmatically? Any why not move away from using global variables to do it?

I wouldn't describe what ExtensionRegistry does as "manipulating configuration settings" - it just sets them based on *static* values and moves on. And in theory, that's all that people do in LocalSettings/CommonSettings too, just define, statically, what the value of configuration settings should be. And those values shouldn't be overridden or futher manipulated at the $wg layer. They might get mutated at runtime, and that's totally fine. Both ExtensionRegistry and InitialiseSettings both go to decent lengths to implement caches based on the understanding that values are static, so any further manipulation during initialization works against those perf gains.

The realistic alternative today is doing the same in the MediaWikiServices hook, which works, but obscures what's happening, since in theory that hook has a different purpose. We should use a dedicated hook instead.

This is, frankly, an issue that has plagued several aspects of our configuration system (e.g. T250406): proposed changes are blocked based on a roadmap that is very nice in theory, but nobody owns it, nobody works on it, and it hasn't really made any progress in the last five or so years. I think this is an antipattern. We should have a preference for imperfect things that exist over perfect things that don't exist but maybe in another five year will. If the perfect thing actually comes into existence, it's still possible to convert and remove the imperfect solution.

Change 703011 abandoned by Daniel Kinzler:

[mediawiki/core@master] Introduce MediaWikiConfigHook

Reason:

registration callbacks are preferrable for this use case.

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