Specify $wgWBRepoSettings['conceptBaseUri']
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Lucas_Werkmeister_WMDE
	Jun 6 2019, 1:36 PM

Description

Wikibase’s default value for the conceptBaseUri setting is correct for our production systems, but computed using a callback, and I dimly remember @hoo saying that this would cause debug log warnings and we should expliclitly specify the correct value in mediawiki-config instead.

In I8f6a6d67d7, I tried to implement this, but this ended up causing an incident and the change had to be reverted. There were at least two errors (fatalmonitor):

On Commons:

Wikimedia\Assert\ParameterTypeException from line 89 of /srv/mediawiki/php-1.34.0-wmf.8/vendor/wikimedia/assert/src/Assert.php: Bad value for parameter $conceptBaseUri: must be a string

This was a simple oversight by me – I failed to specify the variable for commonswiki (and testcommonswiki would presumably also need it).

Unknown origin:

Notice: Undefined variable: wmgWBRepoConceptBaseUri in /srv/mediawiki/wmf-config/Wikibase.php on line 149

These log messages have a “-” in the “wiki” column and not much other useful information either, and I don’t really understand where they’re coming from.

If we want to try this change again, we need to investigate and resolve the second error first.

Details

Subject	Repo	Branch	Lines +/-
Specify $wgWBRepoSettings['conceptBaseUri'] again	operations/mediawiki-config	master	+1 -0
Specify $wmgWBRepoConceptBaseUri again	operations/mediawiki-config	master	+14 -0
Specify $wgWBRepoSettings['conceptBaseUri'] again	operations/mediawiki-config	master	+15 -0

Customize query in gerrit

Related Objects

Mentioned In: T225574: The SiteConfiguration class (wgConf) should not skip extraction if default is specified as null
Mentioned Here: T225574: The SiteConfiguration class (wgConf) should not skip extraction if default is specified as null

Event Timeline

Lucas_Werkmeister_WMDE created this task.Jun 6 2019, 1:36 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 6 2019, 1:36 PM

Lucas_Werkmeister_WMDE updated the task description. (Show Details)Jun 6 2019, 1:40 PM

I dimly remember @hoo saying that this would cause debug log warnings and we should expliclitly specify the correct value in mediawiki-config instead.

I'm not sure what level exactly it logs at, but it at very least debug logs… and given we resolve this on almost every Wikidata related request, we should try to avoid this also for performance reasons (even if no one listens to that log).

Notice: Undefined variable: wmgWBRepoConceptBaseUri in /srv/mediawiki/wmf-config/Wikibase.php on line 149

This is because the default was set to null and that makes SiteConfiguration::getAll (which mediawiki-config uses to propagate the setting globals) omit the variable completely, so this is just another symptom of the incomplete set of wikis this was defined for.

I'm not sure what level exactly it logs at, but it at very least debug logs… and given we resolve this on almost every Wikidata related request, we should try to avoid this also for performance reasons (even if no one listens to that log).

Found it, it’s in SettingsArray::getSetting():

$this->logger->debug(
    '{method}: setting {settingName} was given as a closure, resolve it to {logValue}',
    [
        'method' => __METHOD__,
        'settingName' => $settingName,
        'logValue' => $logValue,
    ]
);

Notice: Undefined variable: wmgWBRepoConceptBaseUri in /srv/mediawiki/wmf-config/Wikibase.php on line 149

This is because the default was set to null and that makes SiteConfiguration::getAll (which mediawiki-config uses to propagate the setting globals) omit the variable completely, so this is just another symptom of the incomplete set of wikis this was defined for.

Ah, thanks.

In T225212#5241912, @hoo wrote:

Notice: Undefined variable: wmgWBRepoConceptBaseUri in /srv/mediawiki/wmf-config/Wikibase.php on line 149

This is because the default was set to null and that makes SiteConfiguration::getAll [..] omit the variable completely.

This seems like something we should fix. Variable existence is not something we normally use intentionally to inform behaviour, so I think SiteConfiguration should always export the variable. On top of that, the patch even explicitly specified 'default' => null which means even if we were to support variables that only conditionally exist, this would be a condition in which it should exist.

(Filed T225574.)

Krinkle mentioned this in T225574: The SiteConfiguration class (wgConf) should not skip extraction if default is specified as null.Jun 11 2019, 10:07 PM

Change 518239 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[operations/mediawiki-config@master] Specify $wgWBRepoSettings['conceptBaseUri'] again

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

gerritbot added a project: Patch-For-Review.Jun 21 2019, 11:39 AM

Addshore moved this task from incoming to in progress on the Wikidata board.Jun 21 2019, 11:25 PM

Change 522125 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[operations/mediawiki-config@master] Specify $wmgWBRepoConceptBaseUri again

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

Change 522126 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[operations/mediawiki-config@master] Specify $wgWBRepoSettings['conceptBaseUri'] again

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

Change 518239 abandoned by Lucas Werkmeister (WMDE):
Specify $wgWBRepoSettings['conceptBaseUri'] again

Reason:
split into Ib3584f456e and I04d74a42c4

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

Change 522125 merged by jenkins-bot:
[operations/mediawiki-config@master] Specify $wmgWBRepoConceptBaseUri again

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

Mentioned in SAL (#wikimedia-operations) [2019-07-15T12:00:40Z] <lucaswerkmeister-wmde@deploy1001> Synchronized wmf-config/: SWAT: [[gerrit:522125|Specify $wmgWBRepoConceptBaseUri again (T225212)]] (duration: 00m 51s)

Change 522126 merged by jenkins-bot:
[operations/mediawiki-config@master] Specify $wgWBRepoSettings['conceptBaseUri'] again

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

Mentioned in SAL (#wikimedia-operations) [2019-07-15T12:07:15Z] <lucaswerkmeister-wmde@deploy1001> Synchronized wmf-config/Wikibase.php: SWAT: [[gerrit:522126|Specify $wgWBRepoSettings['conceptBaseUri'] again (T225212)]] (duration: 00m 50s)

Maintenance_bot removed a project: Patch-For-Review.Jul 15 2019, 12:10 PM

Merged and deployed, and so far everything looks fine – only a single log message looks suspicious:

[XSxscQpAICgAAK13kMMAAABE] /wiki/Q4742980 Wikimedia\Assert\ParameterTypeException from line 89 of /srv/mediawiki/php-1.34.0-wmf.13/vendor/wikimedia/assert/src/Assert.php: Bad value for parameter $conceptBaseUri: must be a string

I’m not sure where that comes from. I SSHed into that app server and ran \Wikibase\Repo\WikibaseRepo::getDefaultInstance()->getSettings()->getSetting( 'conceptBaseUri' ) and the value looked okay, and there haven’t been any other occurrences of this error since then, so I can only conclude that, whatever this was, it seems to have resolved itself. (This looks like an error that might happen if Wikibase.php was synced before InitialiseSettings.php, but I made sure to sync InitialiseSettings.php first, even in a separate change.)

Specify $wgWBRepoSettings['conceptBaseUri']Closed, ResolvedPublicActions

Description

Details

Related Objects

Event Timeline

Specify $wgWBRepoSettings['conceptBaseUri']
Closed, ResolvedPublic
Actions