Page MenuHomePhabricator

Prepare Wikidata descriptions to roll out to stable
Closed, ResolvedPublic3 Story Points

Description

A description shows under the title in beta:

See blocking tasks for what's stopping us pushing this to stable.


Implementation details

  • Move header html from SkinMinervaBeta to SkinMinerva
  • Remove references to isBeta when adding the property to the parser output
  • Update readme to remove references to beta regarding the config variable
  • Change the config default to disable the feature by default
  • Enable feature on beta cluster for testing
  • Test how cached pages appear and post a screenshot on the card.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

[...] seems that if the feature flag is disabled, then we don't remove the wgMFDescription property from the parser output. We should do our best to tidy up after ourselves.

@phuedx what am I missing? I've checked and it seems that if MFUseWikibaseDescription is false we don't add the property to the parser output.


Regarding impl, this is what I see:

Is there anything else TODO? I'm going to tentatively add it to the description.

Jhernandez updated the task description. (Show Details)May 12 2016, 7:41 AM

I've also added Change wm config to disable the feature by default and enable on first rollout wikis.

Do we want to have this feature disabled by default on MobileFrontend proper? Or enabled by default in MF, disabled for all wm wikis, and enable per wiki as we rollout? (with the intention of removing the WM config once the adoption is spread)

Do we want to have this feature disabled by default on MobileFrontend proper? Or enabled by default in MF, disabled for all wm wikis, and enable per wiki as we rollout? (with the intention of removing the WM config once the adoption is spread)

Disabled by default.

[...] seems that if the feature flag is disabled, then we don't remove the wgMFDescription property from the parser output. We should do our best to tidy up after ourselves.

@phuedx what am I missing? I've checked and it seems that if MFUseWikibaseDescription is false we don't add the property to the parser output.

While the feature is enabled, we're adding metadata to the parser output. While the feature is disabled, we're not using the metadata but it might still be there, until the page is edited again. We need to be clear about whether we're going to actively remove the metadata from the parser output.

Jhernandez updated the task description. (Show Details)May 12 2016, 4:42 PM
Jhernandez renamed this task from Wikidata descriptions to stable to Prepare Wikidata descriptions to roll out to stable.May 16 2016, 4:50 PM
Jdlrobson renamed this task from Prepare Wikidata descriptions to roll out to stable to Wikidata descriptions to stable.May 16 2016, 4:50 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson renamed this task from Wikidata descriptions to stable to Prepare Wikidata descriptions to roll out to stable.

We're going to use this task for preparing the beta feature to roll out to stable.

Please create followup tasks blocked by this one to enable the feature on specific wikis (ping @JKatzWMF @dr0ptp4kt @Moushira @Lydia_Pintscher)

MBinder_WMF changed the point value for this task from 2 to 3.May 16 2016, 4:52 PM

@JKatzWMF @Moushira @Lydia_Pintscher, so would this be the proposed rollout?

  1. Roll to Spanish, Portuguese, and Polish for a week
  2. Then roll to the rest. (Also, what is the rest?)

To be clear, this task (let's call it (0)) here is the part that makes it possible to do (1) and (2). That is to say we'd probably need a week or two between (0) task being merged & deployed and (1) happening.

@dr0ptp4kt @Jhernandez Sounds good to me! Thanks.

Regarding #2 is it only mdot Wikipedias?

I'm moving this to sprint 74, in recognition of slightly higher Hovercards bugfixes load than expected.

Oliv0 added a subscriber: Oliv0.Jun 1 2016, 10:26 AM
Oliv0 added a subscriber: Dbrant.EditedJun 1 2016, 4:11 PM

It seems like what we have here is a conflict of use cases. According to the official guidelines, the purpose of the description is to "disambiguate items with the same or similar labels," which is subtly but crucially different from our use case of "a one-line summary of the subject."

This seems to be an even bigger concern than forcing mobile web Wikipedias to use Wikidata descriptions.

Change 293882 had a related patch set uploaded (by Jhobs):
Prepare Wikidata descriptions for rollout to stable

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

Change 293883 had a related patch set uploaded (by Jhobs):
Prepare Wikidata descriptions on mobile for production rollout

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

@jhobs can you also do the last bullet point?

@jhobs can you also do the last bullet point?

Of course, just ran out of time last week and wanted to get it in code review. If I'm not mistaken though, won't we need to SWAT deploy the labs patch first? If not, I may be misunderstanding the the last bullet point (I assume it means "screenshot cached pages after the change has been implemented."

You can test how your patch affects the cached pages by following these steps:

  1. ./dev-scripts/cachedpage.sh 6d588b23ddab1a2cb1514d97018a8213063153ad San_Francisco <- The command fetches the San Francisco article for the commit 6d588b23ddab1a2cb1514d97018a8213063153ad which is before your change.
  2. The above command creates a file called /tmp/cached.html which you can access using the browser. While accessing the file, make sure your patch is active.

You can test how your patch affects the cached pages by following these steps:

  1. ./dev-scripts/cachedpage.sh 6d588b23ddab1a2cb1514d97018a8213063153ad San_Francisco <- The command fetches the San Francisco article for the commit 6d588b23ddab1a2cb1514d97018a8213063153ad which is before your change.
  2. The above command creates a file called /tmp/cached.html which you can access using the browser. While accessing the file, make sure your patch is active.

The second step above does not occur when I run the command. The script executes "successfully" and says the file has been generated, but no such file exists.

@jhobs /tmp/cached.html is relative to the MF extension directory, not the system dir. Mybad.

@jhobs /tmp/cached.html is relative to the MF extension directory, not the system dir. Mybad.

Yeah, I understood that. The tmp folder in my extension directory is empty.

phuedx added a comment.EditedJun 16 2016, 1:13 PM

Yeah, I understood that. The tmp folder in my extension directory is empty.

Weird! This worked perfectly for me.

Change 293882 merged by jenkins-bot:
Prepare Wikidata descriptions for rollout to stable

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

@jhobs I think your config file is ready to be SWAT deployed.

@bmansurov thanks. I'll talk with you guys at standup to see when a good time to schedule it is.

Can someone else upload the cached page screenshot? I was finally able to get the dev script working with the help of Baha, but I can't view the page within Mediawiki for some reason (maybe I need to wipe vagrant...).

It looks like the SWAT deploy is going to happen on Tuesday 2016-06-21. With that in mind, I'm moving this back to RfS since it's been verified.

Check that, it's happening tonight. Thanks @dr0ptp4kt!

Test how cached pages appear and post a screenshot on the card.

👍

Change 293883 merged by jenkins-bot:
Prepare Wikidata descriptions on mobile for production rollout

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

Mentioned in SAL [2016-06-16T23:07:40Z] <ebernhardson@tin> Synchronized wmf-config/InitialiseSettings-labs.php: T127250: Prepare Wikidata descriptions on mobile for production rollout (duration: 00m 27s)

Okay, @bmansurov and @jhobs and I spoke this afternoon. As noted in the patch:

* Note: A follow-up deployment will happen on Monday to disable
*       this by default on production. It is not included in
*       this patch in order to minimize the time in prod-beta
*       without wikibase descriptions.

Some additional code on top of the existing MASTER is also needed to add logic to ensure beta will generally retain Wikidata descriptions.

^ Heads up @Lydia_Pintscher there may be a brief period next week on beta mobile web Wikipedias where you wouldn't see Wikidata descriptions, but work will be in flight to keep that period short.

@dr0ptp4kt do we have follow-up tasks for:

A follow-up deployment will happen on Monday to disable this by default on production

and

Some additional code on top of the existing MASTER is also needed to add logic to ensure beta will generally retain Wikidata descriptions.

?

@Jhernandez, I'll file the Task for the SWAT.

@jhobs is going to fix the conditional logic via this task to make it complete.

Change 295012 had a related patch set uploaded (by Jhobs):
Enable wikibase descriptions for beta, even if disabled for production

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

Change 295012 merged by jenkins-bot:
Enable wikibase descriptions for beta, even if disabled for production

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

Change 295311 had a related patch set uploaded (by Jhobs):
Remove old mobile workaround for wikidata descriptions

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

Change 295311 merged by jenkins-bot:
Remove old mobile workaround for wikidata descriptions

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

Mentioned in SAL [2016-06-20T23:13:23Z] <dereckson@tin> Synchronized wmf-config/mobile.php: Remove old mobile workaround for Wikidata descriptions (T127250, T138085) (duration: 00m 33s)

phuedx closed this task as Resolved.Jun 21 2016, 8:34 AM

All of the AC have been met. Ensuring that this feature is disabled by default but enabled on test wikis is covered by T138085.