Page MenuHomePhabricator

Newcomer tasks: pageview count not appearing or inconsistent
Closed, ResolvedPublic

Description

On each suggested edits card, the pageview count of the article should appear, like this screenshot from beta:

The issue is that the pageviews only show up on some articles in beta and no articles in production (Update: @Catrope found a few in cswiki with pageviews). This is cswiki:

Details

Related Gerrit Patches:
mediawiki/extensions/GrowthExperiments : wmf/1.35.0-wmf.8Suggested edits: do not treat AQS lookup failure as error
mediawiki/extensions/GrowthExperiments : masterSuggested edits: do not treat AQS lookup failure as error
mediawiki/extensions/GrowthExperiments : masterUse AQS for fetching pageview data

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 13 2019, 1:50 AM
MMiller_WMF renamed this task from Newcomer tasks: pageview count not appearing or inconsisten to Newcomer tasks: pageview count not appearing or inconsistent.Nov 13 2019, 1:51 AM
MMiller_WMF updated the task description. (Show Details)
Tgr added a comment.Nov 20 2019, 5:20 AM

This is some kind of PageViewInfo bug that's somehow related to API continuation (continuation is incorrectly triggered in the pageviewinfo module, even for very small result sets like 10 pages).
I can look into it tomorrow, or we could hack around it temporarily by using pageviews data from the RESTBase/PCS summary endpoint instead.

Change 552148 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/GrowthExperiments@master] [WIP] Use AQS for fetching pageview data

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

Change 552148 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Use AQS for fetching pageview data

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

@Tgr one downside to this approach is that if the call to the Pageviews service errors out, the entire card fails to render. It would be nice if the pageview data was treated as an extra component, rather than a blocker to rendering the card.

Another nice-to-have with this for local development and QA would be to parse GERestbaseUrl, if set, and extract the domain (e.g. cs.wikipedia.org) to use there, rather than relying on wgServer.

@Tgr -is this config setup missing?

Another nice-to-have with this for local development and QA would be to parse GERestbaseUrl, if set, and extract the domain (e.g. cs.wikipedia.org) to use there, rather than relying on wgServer.

What I observe on betalabs - the initial load of Suggested edits fails displaying suggestions, but the navigation arrow (the right hand one) is active.

Clicking on the right hand arrow produces two requests, e.g.
https://en.wikipedia.beta.wmflabs.org/api/rest_v1/page/summary/St%C5%99edn%C3%AD_Brdy

https://wikimedia.org/api/rest_v1/metrics/pageviews/per-article/en.wikipedia.org/all-access/user/St%C5%99edn%C3%AD_Brdy/monthly/20190924/20191123 404

After couple of clicks, the article card would be displayed displayed correctly.

Tgr added a comment.Nov 26 2019, 4:30 AM

@Tgr one downside to this approach is that if the call to the Pageviews service errors out, the entire card fails to render.

Yup, that needs fixing. It doesn't even have to error as AQL unhelpfully sends a 404 if the page has zero views...

Another nice-to-have with this for local development and QA would be to parse GERestbaseUrl, if set, and extract the domain (e.g. cs.wikipedia.org) to use there, rather than relying on wgServer.

The domain is taken from $wgPageViewInfoWikimediaDomain (if set). That's less discoverable, but more relevant, and you might want to use a local RESTBase instance (for testing with local articles; "local" might also mean beta) but non-local AQS (as setting that up is even more of a PITA than RESTBase).

MMiller_WMF assigned this task to Tgr.Nov 26 2019, 3:58 PM
MMiller_WMF moved this task from QA to In Progress on the Growth-Team (Current Sprint) board.
kostajh added a comment.EditedNov 26 2019, 4:00 PM

The domain is taken from $wgPageViewInfoWikimediaDomain (if set). That's less discoverable, but more relevant, and you might want to use a local RESTBase instance (for testing with local articles; "local" might also mean beta) but non-local AQS (as setting that up is even more of a PITA than RESTBase).

Sorry, I meant specifically here, to have an override for get( 'ServerName') when using this locally.

'GrowthExperimentsEditInfoService' => function ( MediaWikiServices $services ): EditInfoService {
		$editInfoService = new AqsEditInfoService( $services->getHttpRequestFactory(),
			$services->getMainConfig()->get( 'ServerName' ) );
		$editInfoService->setCache( ObjectCache::getLocalClusterInstance() );
		return $editInfoService;
	},
Tgr added a comment.Nov 26 2019, 4:52 PM

Locally, I use

$wgHooks['MediaWikiServices'][] = function ( MediaWikiServices $services ) {
        if ( !$services->has( 'GrowthExperimentsEditInfoService' ) ) return;
        $services->redefineService( 'GrowthExperimentsEditInfoService', function ( MediaWikiServices $services ) {
                return new AqsEditInfoService( $services->getHttpRequestFactory(), 'cs.wikipedia' );
        } );
};

for that. It's only used in the mobile module summary footer which says something about edits per day, and will ignore errors anyway, but it would certainly make sense to use an override here if we use it elsewhere...

Change 553175 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/GrowthExperiments@master] Suggested edits: do not treat AQS lookup failure as error

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

Ah, right, I can override the AQS Config service:

$wgHooks['MediaWikiServices'][] = function ( MediaWikiServices $services ) {
	if ( $services->hasService( '_GrowthExperimentsAQSConfig' ) ) {
		$services->redefineService( '_GrowthExperimentsAQSConfig',
			function ( MediaWikiServices $services ) {
				$project = 'cs.wikipedia.org';
				if ( ExtensionRegistry::getInstance()->isLoaded( 'PageViewInfo' ) ) {
					$project = $services->getConfigFactory()->makeConfig( 'PageViewInfo' )
						->get( 'PageViewInfoWikimediaDomain' )
						?: $project;
				}
				return (object)[ 'project' => $project ];
			} );
	}
};

Change 553175 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested edits: do not treat AQS lookup failure as error

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

Tgr added a comment.Nov 26 2019, 10:33 PM

Ah, right, I can override the AQS Config service:

You can do that, but you can also just set $wgPageViewInfoWikimediaDomain.

...although I guess that only works if you have PageViewInfo installed. Maybe we should change that?
In theory, you cannot access the configuration setting of an extension unless that extension is installed. In practice, the whole Config layer is fake and everything is just globals anyway, so we could just fetch it via the main config object.

@Etonkovidova Locally I've verified that this patch does what it says. In order for you to QA on beta, we would have to override $wgPageViewInfoWikimediaDomain. This might end up confusing others. Maybe we could just override it for cs beta. Do you have a preference on this?

Tgr added a comment.Nov 27 2019, 6:49 PM

$wgPageViewInfoWikimediaDomain is already overridden on beta to point to the corresponding production wiki. That should work well for us, except on beta enwiki (since we make it use cswiki for the tasks, but it tries to find pageview data on enwiki). Not sure how much it is used for testing; maybe we should have an English task configuration, and have beta enwiki use that and pull tasks from prod enwiki.

Change 553402 had a related patch set uploaded (by Catrope; owner: Gergő Tisza):
[mediawiki/extensions/GrowthExperiments@wmf/1.35.0-wmf.8] Suggested edits: do not treat AQS lookup failure as error

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

Change 553402 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@wmf/1.35.0-wmf.8] Suggested edits: do not treat AQS lookup failure as error

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

Mentioned in SAL (#wikimedia-operations) [2019-12-02T12:19:50Z] <lucaswerkmeister-wmde@deploy1001> Synchronized php-1.35.0-wmf.8/extensions/GrowthExperiments/: SWAT: [[gerrit:553402|Suggested edits: do not treat AQS lookup failure as error (T238178)]] (duration: 01m 02s)

Etonkovidova closed this task as Resolved.Dec 11 2019, 12:19 AM