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:

image.png (622×438 px, 49 KB)

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:

image.png (631×437 px, 133 KB)

Event Timeline

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)

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.

Screen Shot 2019-11-25 at 3.37.18 PM.png (596×423 px, 54 KB)

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

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;
	},

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

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?

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